fix(dns): validate enrtree-root fields and skip malformed records

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<FormatException>());
+
     [TestCase("enrtree://all.mainnet.ethdisco.net")]
     [TestCase("enrtree://AKA3AM6LPBYEUDMVNU3BSVQJ5AD45Y7YPOHJLEF6W26QOE4VTUDPE@all.mainnet.ethdisco.net")]
     public void Can_parse_linked_tree(string text)

src/Nethermind/Nethermind.Network.Dns/EnrTreeCrawler.cs

@@ -53,7 +53,18 @@ private async IAsyncEnumerable<string> SearchNode(IDnsClient client, string quer
             IEnumerable<string> 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);

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;
+        }
     }
 }