-
-
Notifications
You must be signed in to change notification settings - Fork 74
Description
Description of the bug
UDPClient asserts that the first UDP packet received is the reply to the just-sent query. Under normal network churn or when many queries run concurrently, a stray DNS response (e.g., late/old packet delivered after ephemeral-port reuse) can hit the socket first. That makes the transaction ID mismatch and the assertion throws, crashing the process.
This is a correctness + robustness issue: with UDP, the first packet on a socket isn’t guaranteed to be “ours”.
Steps To Reproduce
Steps
- Set up a small Node app that uses dns2’s UDPClient (udp4/udp6) against a public resolver.
- From that app, issue many DNS queries in quick bursts (hundreds), mixing A and AAAA lookups to the same few popular domains (e.g., google.com, cloudflare.com, github.com).
- Let each query open a fresh UDP socket and close it immediately after the first packet is received (the library’s default pattern).
- Run multiple bursts back-to-back for a few minutes.
- (Optional but helps) Create network churn: toggle a VPN (e.g., WireGuard) or flap the default interface so ephemeral ports get reused and late packets are more likely.
Expected
Queries either succeed or time out without crashing the process.
Actual
Intermittently, the process crashes with an assertion from udp.js complaining that the DNS response transaction ID doesn’t match the query ID (assert.equal(response.header.id, query.header.id)), indicating a stray/late UDP reply was treated as the first message on the new socket.
Additional Information
Debugging
In lib/client/udp.js
(or equivalent), the client:
- Sends a DNS query with some query.header.id (currently limited to ~0–9999).
- Listens for the first 'message' event.
- Immediately assert.equal(response.header.id, query.header.id).
- Closes the socket.
Because UDP is unordered/unreliable and ephemeral ports are quickly reused, a late/stray DNS response can arrive first. That packet’s transaction ID won’t match and the assertion throws. There’s also no verification of sender (rinfo.address/rinfo.port
), so packets from unexpected sources aren’t filtered.
Environment
- dns2 version: 2.1.0
- Node.js: 20.19.4
- OS: Seen on MacOS (likely OS-agnostic)
Suggested fix (backwards-compatible)
Replace the assertion with filtering + timeout
- Verify rinfo.address and rinfo.port against the configured server.
- If Packet.parse(message).header.id !== query.header.id, ignore and keep listening.
- Add a timeout to reject with ETIMEDOUT if the matching response doesn’t arrive.
Use full 16-bit transaction IDs
- query.header.id = crypto.randomInt(0x10000) to reduce collision probability.