diff --git a/src/Nethermind/Nethermind.Network.Dns.Test/EnrTreeParserTests.cs b/src/Nethermind/Nethermind.Network.Dns.Test/EnrTreeParserTests.cs index f470fd3e01e3..8f14dda6dd51 100644 --- a/src/Nethermind/Nethermind.Network.Dns.Test/EnrTreeParserTests.cs +++ b/src/Nethermind/Nethermind.Network.Dns.Test/EnrTreeParserTests.cs @@ -1,6 +1,7 @@ // SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited // SPDX-License-Identifier: LGPL-3.0-only +using System; using NUnit.Framework; namespace Nethermind.Network.Dns.Test; @@ -41,6 +42,21 @@ public void Can_parse_leaf() Assert.That(leaf.NodeRecord, Is.EqualTo(enr.Substring(4))); } + // Malformed input must surface as a FormatException the crawler can catch, not an unhandled exception that aborts the crawl. + [TestCase("enrtree-root:v1", TestName = "all fields missing")] + [TestCase("enrtree-root:v1 e=TPLRUM3FAKJZIRMXADWOHSU3PM l=FDXN3SN67NA5DKA4J2GOK7BVQI seq=2779sig=CNoJofW_lNh7QFQkaVGhEX2ifbEZ3UkiBQCVyZCkM_I-72cEh8Bfd21cSS9BP5tyAqWF3jMVov8duUCdSByEQAE", TestName = "no space after seq value")] + [TestCase("enrtree-root:v1 e=TPLRUM3FAKJZIRMXADWOHSU3PM l=FDXN3SN67NA5DKA4J2GOK7BVQI sig=CNoJofW_lNh7QFQkaVGhEX2ifbEZ3UkiBQCVyZCkM_I-72cEh8Bfd21cSS9BP5tyAqWF3jMVov8duUCdSByEQAE", TestName = "seq field missing")] + [TestCase("enrtree-root:v1 e=TPLRUM3FAKJZIRMXADWOHSU3PM l=FDXN3SN67NA5DKA4J2GOK7BVQI seq=2779 sig=SHORT", TestName = "truncated sig field")] + [TestCase("enrtree-root:v1 e=TPLRUM3FAKJZIRMXADWOHSU3PM l=FDXN3SN67NA5DKA4J2GOK7BVQI seq=notanumber sig=CNoJofW_lNh7QFQkaVGhEX2ifbEZ3UkiBQCVyZCkM_I-72cEh8Bfd21cSS9BP5tyAqWF3jMVov8duUCdSByEQAE", TestName = "non-numeric seq value")] + [TestCase("enrtree-root:v1 l=FDXN3SN67NA5DKA4J2GOK7BVQI seq=2779 sig=CNoJofW_lNh7QFQkaVGhEX2ifbEZ3UkiBQCVyZCkM_I-72cEh8Bfd21cSS9BP5tyAqWF3jMVov8duUCdSByEQAE", TestName = "e field missing")] + [TestCase("enrtree-root:v1 e=TPLRUM3FAKJZIRMXADWOHSU3PM seq=2779 sig=CNoJofW_lNh7QFQkaVGhEX2ifbEZ3UkiBQCVyZCkM_I-72cEh8Bfd21cSS9BP5tyAqWF3jMVov8duUCdSByEQAE", TestName = "l field missing")] + [TestCase("enrtree-root:v1 e=TPLRUM3FAKJZIRMXADWOHSU3PM e=TPLRUM3FAKJZIRMXADWOHSU3PM l=FDXN3SN67NA5DKA4J2GOK7BVQI seq=2779 sig=CNoJofW_lNh7QFQkaVGhEX2ifbEZ3UkiBQCVyZCkM_I-72cEh8Bfd21cSS9BP5tyAqWF3jMVov8duUCdSByEQAE", TestName = "duplicate e field")] + [TestCase("enrtree-root:v1 e=TPLRUM3FAKJZIRMXADWOHSU3PM l=FDXN3SN67NA5DKA4J2GOK7BVQI l=FDXN3SN67NA5DKA4J2GOK7BVQI seq=2779 sig=CNoJofW_lNh7QFQkaVGhEX2ifbEZ3UkiBQCVyZCkM_I-72cEh8Bfd21cSS9BP5tyAqWF3jMVov8duUCdSByEQAE", TestName = "duplicate l field")] + [TestCase("enrtree-root:v1 e=TPLRUM3FAKJZIRMXADWOHSU3PM l=FDXN3SN67NA5DKA4J2GOK7BVQI seq=2779 seq=2779 sig=CNoJofW_lNh7QFQkaVGhEX2ifbEZ3UkiBQCVyZCkM_I-72cEh8Bfd21cSS9BP5tyAqWF3jMVov8duUCdSByEQAE", TestName = "duplicate seq field")] + [TestCase("enrtree-root:v1 e=TPLRUM3FAKJZIRMXADWOHSU3PM l=FDXN3SN67NA5DKA4J2GOK7BVQI seq=2779 sig=CNoJofW_lNh7QFQkaVGhEX2ifbEZ3UkiBQCVyZCkM_I-72cEh8Bfd21cSS9BP5tyAqWF3jMVov8duUCdSByEQAE sig=CNoJofW_lNh7QFQkaVGhEX2ifbEZ3UkiBQCVyZCkM_I-72cEh8Bfd21cSS9BP5tyAqWF3jMVov8duUCdSByEQAE", TestName = "duplicate sig field")] + public void Malformed_enrtree_root_throws_FormatException(string malformedRoot) + => Assert.That(() => EnrTreeParser.ParseEnrRoot(malformedRoot), Throws.TypeOf()); + [TestCase("enrtree://all.mainnet.ethdisco.net")] [TestCase("enrtree://AKA3AM6LPBYEUDMVNU3BSVQJ5AD45Y7YPOHJLEF6W26QOE4VTUDPE@all.mainnet.ethdisco.net")] public void Can_parse_linked_tree(string text) diff --git a/src/Nethermind/Nethermind.Network.Dns/EnrTreeCrawler.cs b/src/Nethermind/Nethermind.Network.Dns/EnrTreeCrawler.cs index e3dbfc56ae27..e1bc04ee36c1 100644 --- a/src/Nethermind/Nethermind.Network.Dns/EnrTreeCrawler.cs +++ b/src/Nethermind/Nethermind.Network.Dns/EnrTreeCrawler.cs @@ -53,7 +53,18 @@ private async IAsyncEnumerable SearchNode(IDnsClient client, string quer IEnumerable lookupResult = await client.Lookup(query, cancellationToken); foreach (string node in lookupResult) { - EnrTreeNode treeNode = EnrTreeParser.ParseNode(node); + EnrTreeNode treeNode; + try + { + treeNode = EnrTreeParser.ParseNode(node); + } + catch (Exception e) when (e is FormatException or NotSupportedException) + { + // A single malformed record from an untrusted DNS server must not abort the whole crawl. + if (_logger.IsDebug) _logger.Debug($"Skipping malformed ENR tree record from DNS query '{query}': {e.Message}"); + continue; + } + foreach (string link in treeNode.Links) { DnsClient linkedTreeLookup = new(link); diff --git a/src/Nethermind/Nethermind.Network.Dns/EnrTreeParser.cs b/src/Nethermind/Nethermind.Network.Dns/EnrTreeParser.cs index 1c37cdcdc587..775ebbf5785b 100644 --- a/src/Nethermind/Nethermind.Network.Dns/EnrTreeParser.cs +++ b/src/Nethermind/Nethermind.Network.Dns/EnrTreeParser.cs @@ -63,21 +63,60 @@ public static EnrTreeRoot ParseEnrRoot(string enrTreeRootText) { ArgumentNullException.ThrowIfNull(enrTreeRootText); - EnrTreeRoot enrTreeRoot = new(); - - int ensRootIndex = enrTreeRootText.IndexOf("e=", StringComparison.Ordinal); - enrTreeRoot.EnrRoot = enrTreeRootText.Substring(ensRootIndex + 2, HashLengthBase32); - - int linkRootIndex = enrTreeRootText.IndexOf("l=", StringComparison.Ordinal); - enrTreeRoot.LinkRoot = enrTreeRootText.Substring(linkRootIndex + 2, HashLengthBase32); + // enrTreeRootText comes from an untrusted DNS TXT record (parsed before any signature check), + // so validate every field's presence and length before slicing to avoid an unhandled exception. + EnrTreeRoot enrTreeRoot = new() + { + EnrRoot = ExtractFixedField(enrTreeRootText, "e=", HashLengthBase32), + LinkRoot = ExtractFixedField(enrTreeRootText, "l=", HashLengthBase32), + Sequence = ExtractSequence(enrTreeRootText), + Signature = ExtractFixedField(enrTreeRootText, "sig=", SigLengthBase32), + }; - int seqIndex = enrTreeRootText.IndexOf("seq=", StringComparison.Ordinal); - int seqLength = enrTreeRootText.IndexOf(' ', seqIndex) - (seqIndex + 4); - enrTreeRoot.Sequence = int.Parse(enrTreeRootText.AsSpan(seqIndex + 4, seqLength)); + return enrTreeRoot; - int sigIndex = enrTreeRootText.IndexOf("sig=", StringComparison.Ordinal); - enrTreeRoot.Signature = enrTreeRootText.Substring(sigIndex + 4, SigLengthBase32); + static string ExtractFixedField(string text, string key, int length) + { + int index = text.IndexOf(key, StringComparison.Ordinal); + if (index < 0 || index + key.Length + length > text.Length) + { + throw new FormatException($"Malformed enrtree-root: '{key}' field is missing or too short."); + } + + if (text.IndexOf(key, index + key.Length, StringComparison.Ordinal) >= 0) + { + throw new FormatException($"Malformed enrtree-root: '{key}' field appears more than once."); + } + + return text.Substring(index + key.Length, length); + } - return enrTreeRoot; + static int ExtractSequence(string text) + { + int index = text.IndexOf("seq=", StringComparison.Ordinal); + if (index < 0) + { + throw new FormatException("Malformed enrtree-root: 'seq=' field is missing."); + } + + if (text.IndexOf("seq=", index + "seq=".Length, StringComparison.Ordinal) >= 0) + { + throw new FormatException("Malformed enrtree-root: 'seq=' field appears more than once."); + } + + int start = index + "seq=".Length; + int end = text.IndexOf(' ', start); + if (end < 0) + { + throw new FormatException("Malformed enrtree-root: 'seq=' field is not terminated."); + } + + if (!int.TryParse(text.AsSpan(start, end - start), out int sequence)) + { + throw new FormatException("Malformed enrtree-root: 'seq=' value is not a valid number."); + } + + return sequence; + } } }