Skip to content

common/domain: avoid pre-allocating untrusted lengths in the set reader - #107

Open
AlexandrKhromov2005 wants to merge 1 commit into
SagerNet:devfrom
AlexandrKhromov2005:fix/domain-unbounded-allocation
Open

common/domain: avoid pre-allocating untrusted lengths in the set reader#107
AlexandrKhromov2005 wants to merge 1 commit into
SagerNet:devfrom
AlexandrKhromov2005:fix/domain-unbounded-allocation

Conversation

@AlexandrKhromov2005

Copy link
Copy Markdown

common/domain: avoid pre-allocating untrusted lengths in the set reader

readUint64Slice and readByteSlice read a length with binary.ReadUvarint and immediately
allocate a slice of that size (make([]uint64, length) / make([]byte, length)) before reading
the data. The length is untrusted — these readers parse serialized matcher data, reachable e.g.
from sing-box .srs rule-sets via AdGuard-format domain items — so a crafted input can declare a
huge length and either OOM the process on the allocation or panic with
makeslice: len out of range.

Found by fuzzing (go test -fuzz).

Fix

Read incrementally instead of trusting the length for a single allocation:

  • []uint64 grows via append with a capped initial capacity, so a bogus length hits EOF while
    reading the elements;
  • []byte goes through io.CopyN into a growable buffer, which only grows as bytes actually
    arrive.

Adds FuzzReadMatcher / FuzzReadAdGuardMatcher. Existing tests pass; valid data still decodes.

Companion to SagerNet/sing-box#4399, which fixes the same pattern in the .srs reader.

readUint64Slice and readByteSlice read a length via binary.ReadUvarint and then
immediately allocate a slice of that size before reading the data. The length is
untrusted (these readers parse serialized matcher data, reachable e.g. from
sing-box .srs rule-sets via AdGuard domain items), so a crafted input can
declare a huge length and OOM the process on the allocation, or panic with
"makeslice: len out of range".

Read incrementally instead: uint64 slices grow via append with a capped initial
capacity; byte slices go through io.CopyN so the buffer grows only as bytes
actually arrive. Add FuzzReadMatcher / FuzzReadAdGuardMatcher.
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.

1 participant