Skip to content

fixed crash of out-of-bounds Buffer() access with malformed requests:#102

Merged
lsongdev merged 1 commit into
lsongdev:masterfrom
lif-zone:master
May 25, 2026
Merged

fixed crash of out-of-bounds Buffer() access with malformed requests:#102
lsongdev merged 1 commit into
lsongdev:masterfrom
lif-zone:master

Conversation

@lif-rnd
Copy link
Copy Markdown

@lif-rnd lif-rnd commented May 24, 2026

RangeError [ERR_BUFFER_OUT_OF_BOUNDS]: Attempt to access memory outside buffer bounds
at boundsError (node:internal/buffer:90:11)
at readUInt8 (node:internal/buffer:258:5)
at Buffer. (node:internal/buffer:1009:47)
at BufferReader.read (/home/lif/lif-kernel/node_modules/dns2/lib/reader.js:38:24)
at BufferReader.read (/home/lif/lif-kernel/node_modules/dns2/lib/reader.js:48:28)
at Packet.Header.parse (/home/lif/lif-kernel/node_modules/dns2/packet.js:214:22)
at Packet.parse (/home/lif/lif-kernel/node_modules/dns2/packet.js:116:33)
at Server.handle (/home/lif/lif-kernel/node_modules/dns2/server/tcp.js:15:28)
at process.processTicksAndRejections (node:internal/process/task_queues:104:5) {
code: 'ERR_BUFFER_OUT_OF_BOUNDS'

Closes #

📑 Description

✅ Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

ℹ Additional Information

RangeError [ERR_BUFFER_OUT_OF_BOUNDS]: Attempt to access memory outside buffer bounds
    at boundsError (node:internal/buffer:90:11)
    at readUInt8 (node:internal/buffer:258:5)
    at Buffer.<anonymous> (node:internal/buffer:1009:47)
    at BufferReader.read (/home/lif/lif-kernel/node_modules/dns2/lib/reader.js:38:24)
    at BufferReader.read (/home/lif/lif-kernel/node_modules/dns2/lib/reader.js:48:28)
    at Packet.Header.parse (/home/lif/lif-kernel/node_modules/dns2/packet.js:214:22)
    at Packet.parse (/home/lif/lif-kernel/node_modules/dns2/packet.js:116:33)
    at Server.handle (/home/lif/lif-kernel/node_modules/dns2/server/tcp.js:15:28)
    at process.processTicksAndRejections (node:internal/process/task_queues:104:5) {
  code: 'ERR_BUFFER_OUT_OF_BOUNDS'
@lsongdev lsongdev merged commit ad12792 into lsongdev:master May 25, 2026
0 of 2 checks passed
@msimerson
Copy link
Copy Markdown
Contributor

This change is bad. It is why the test suite is now hanging:

Packet.parse is now swallowing header errors (packet.js:151-156)

When the DOH server receives ?dns=INVALID, it decodes to 5 bytes — too short for the 12-byte DNS header. Header.parse throws a RangeError, but the surrounding try/catch silently returned an empty Packet instead of re-throwing. The DOH server then emits request (not requestError) with that empty packet. The no-op handler never sent a response, leaving the HTTP connection open forever and blocking await server.close().

This change / PR should be reverted.

@msimerson
Copy link
Copy Markdown
Contributor

The reporter hit ERR_BUFFER_OUT_OF_BOUNDS inside Packet.Header.parse from a malformed TCP request. Their stack ends at process.processTicksAndRejections with no catch frame — meaning their node_modules/dns2/server/tcp.js did not wrap Packet.parse in try/catch, so the rejection went unhandled and crashed Node.

That wrapper has been in master since 2021-12-13 (e4877a0, "Handle & report server request errors as 'requestError' events"), and lives in all three transports today (server/tcp.js:14, server/udp.js:23, server/doh.js:46). The reporter was simply on an outdated release.

Why ad12792 is the wrong fix.

  1. Wrong layer. The boundary already exists at every transport. ad12792 duplicates it deeper inside Packet.parse, which solves nothing for current users and only partially helps the reporter.
  2. Replaces one crash with another. After catching, it return packet with packet.header still undefined. The next statement (packet.header.qdcount at packet.js:153) then throws TypeError: Cannot read properties of undefined. In modern code that re-throws into the transport-level catch (net zero); on the reporter's stale code it just relocates the unhandled rejection.
  3. Incomplete. Only Header.parse is wrapped. Question.parse and Resource.parse can throw the same ERR_BUFFER_OUT_OF_BOUNDS on a 12-byte-header-plus-junk packet — still crashes.
  4. Silences a signal. Even if (1–3) were fixed, demoting parse failures to debug() hides hostile probes. The existing transport handlers already surface them as requestError, which is the right channel.

Right solution. Revert it and tell affected users to upgrade past e4877a0.

@msimerson msimerson mentioned this pull request May 25, 2026
4 tasks
msimerson added a commit to NicTool/node-dns that referenced this pull request May 25, 2026
lsongdev pushed a commit that referenced this pull request May 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants