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

[AnkushinDaniil]

Changes

• EnrTreeParser.ParseEnrRoot now validates the presence and length of all four fields (e=, l=, seq=, sig=) in an untrusted DNS TXT record before slicing, and uses int.TryParse for the sequence. Previously a malformed record (missing field, no space after the seq= value, truncated sig=, non-numeric seq=) threw an unhandled ArgumentOutOfRangeException/FormatException from the raw IndexOf/Substring calls. Because this parsing happens before any ENR signature verification, a spoofed or malformed record could abort the entire DNS-discovery crawl.
• EnrTreeCrawler.SearchNode now catches a parse failure for a single record (FormatException/NotSupportedException/ArgumentException), logs it at debug, and continues the crawl instead of aborting it.
• Added regression tests covering the malformed-root cases.

Types of changes

What types of changes does your code introduce?

• Bugfix (a non-breaking change that fixes an issue)

Testing

Requires testing

• Yes

If yes, did you write tests?

• Yes

Notes on testing

New Malformed_enrtree_root_throws_FormatException cases fail before the change (unhandled ArgumentOutOfRangeException) and pass after. Full Nethermind.Network.Dns.Test suite passes.

Documentation

Requires documentation update

• No

Requires explanation in Release Notes

• No

[claude]

Claude finished @AnkushinDaniil's task in 5m 23s —— View job

---

Code Review

• Gather context and read changed files
• Review EnrTreeParser.cs changes
• Review EnrTreeCrawler.cs changes
• Review EnrTreeParserTests.cs changes
• Post findings

---

Summary

The fix is correct and addresses a real reliability/security issue: a malformed DNS TXT record could throw ArgumentOutOfRangeException or FormatException before signature verification, aborting the entire DNS-discovery crawl. The new ExtractFixedField / ExtractSequence local functions are clean, the error messages are clear, and the regression tests cover all the key failure modes described in the PR.

One medium finding on the exception catch scope, and one low finding on test coverage.

---

Findings

Medium

EnrTreeCrawler.cs:61 — ArgumentException catch is broader than necessary (see inline comment)

ArgumentException is a parent class that covers ArgumentNullException, ArgumentOutOfRangeException, and others. Now that ParseEnrRoot converts all data-path exceptions to FormatException, the only exceptions ParseNode can legitimately produce are FormatException (malformed data) and NotSupportedException (unknown node type). Catching ArgumentException is now redundant and risks swallowing programming bugs (e.g. a future ArgumentNullException from a bad code change would be silently logged at Debug and skipped instead of surfacing as a crash). Suggest removing ArgumentException from the catch filter.

Low

EnrTreeParserTests.cs:47-53 — missing isolated tests for e= and l= missing (see inline comment)

Both fields share the same ExtractFixedField code path, so their "missing field" failure mode is only exercised implicitly via the "all fields missing" case. Adding one TestCase each for e= missing and l= missing in isolation would make the coverage explicit and guard against future field-specific regressions.

Informational (pre-existing, not introduced by this PR)

• SigLengthBase32 = 87 — constant name says "Base32" but the signature uses base64url encoding. Not introduced here, but worth cleaning up at some point.
• Sequence is int (32-bit signed) — EIP-1459 specifies seq as a uint64. Pre-existing limitation; no regression from this PR.

---

Critical: 0 | High: 0 | Medium: 1 | Low: 1

[AnkushinDaniil]

Addressed both: dropped ArgumentException from the crawler catch (all data-validation errors now surface as FormatException, so it was redundant and could have masked a genuine ArgumentNullException), and added isolated e=/l=-missing test cases.

[claude]

Claude finished @AnkushinDaniil's task in 1m 33s —— View job

---

Code Review (follow-up)

• Load tools
• Read current state of changed files
• Verify Medium fix: ArgumentException removed from catch
• Verify Low fix: e=/l= isolated test cases added
• Check for any new issues
• Post findings

---

Summary

Both findings from the previous review are confirmed resolved.

Medium — ArgumentException catch (EnrTreeCrawler.cs:61): Fixed. The catch filter is now when (e is FormatException or NotSupportedException) — the ArgumentException hierarchy is gone.

Low — missing e=/l= isolated tests (EnrTreeParserTests.cs:51-52): Fixed. Two new TestCases cover each field missing in isolation.

No new issues found. The ExtractFixedField/ExtractSequence local-function design is sound — field keys use lowercase characters and the separator =, which cannot appear inside base32 hash values or unpadded base64url signature values, so there's no risk of a false IndexOf match inside another field's value.

Pre-existing informational notes from the prior review remain unchanged (both pre-date this PR):

• SigLengthBase32 = 87 — constant name says "Base32" but the signature is base64url-encoded.
• Sequence typed as int (32-bit signed) while EIP-1459 specifies seq as uint64.

Critical: 0 | High: 0 | Medium: 0 | Low: 0 — ready to merge.