Skip to content

src/pack_dns.c: fix out-of-bounds read in generate_dns_values#250

Closed
aizu-m wants to merge 1 commit into
dbry:masterfrom
aizu-m:pack-dns-short-block-overread
Closed

src/pack_dns.c: fix out-of-bounds read in generate_dns_values#250
aizu-m wants to merge 1 commit into
dbry:masterfrom
aizu-m:pack-dns-short-block-overread

Conversation

@aizu-m

@aizu-m aizu-m commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Turned up while fuzzing the encoder. AddressSanitizer, compressing a 15-sample-frame stereo WAV with wavpack -b192:

ERROR: AddressSanitizer: heap-buffer-overflow ... READ of size 4
    #0 generate_dns_values src/pack_dns.c
0x602000000214 is located 0 bytes after 4-byte region [..,0x602000000214)

low_freq and high_freq are sized as filtered_count floats, and filtered_count is sample_count - FILTER_LENGTH + 1 (FILTER_LENGTH is 15). A block of exactly 15 samples makes filtered_count 1, so the delta-filter step that duplicates the unknown first sample reads low_freq[1], one float past the allocation.

Triggered by any hybrid or lossy encode whose only or final block holds 15 sample frames. With a single point there is nothing to duplicate, so the copy is guarded. Round-trips of normal and 15-frame files are unchanged.

@dbry

dbry commented Jun 21, 2026

Copy link
Copy Markdown
Owner

Good catch, thanks! I reproduced this with a 15-sample file, however I think that's the only way to trigger it. The logic around the last buffers in a file prevents very short frames at the end.

The other thing is that having such a small analysis window actually doesn't make any sense. This analyzes filtered audio data, and 1 or 2 samples of filtered audio has no meaning. I decided that a better fix is to simply limit this function to only working when it has at least 15 filtered samples (i.e., sample count > 28). That's what I was thinking when I wrote it, which is why it never occurred to me that filtered_count could be 1 (and it shouldn't be)!

My fix is a92507e

@dbry dbry closed this Jun 21, 2026
@aizu-m

aizu-m commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

Makes sense. Gating on a meaningful analysis window is the cleaner fix; one or two filtered samples carry no information, so guarding the duplication was only treating the symptom. The 15-sample whole-file case was the only trigger I could land too, so limiting the function covers it at the source. Thanks for sorting it.

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.

2 participants