Skip to content

Fix compression-ratio status buffer overflow in CLI tools - #266

Merged
dbry merged 1 commit into
dbry:masterfrom
junkilee80:fix/cli-ratio-format-overflow
Oct 15, 2026
Merged

dbry merged 1 commit into
dbry:masterfrom
junkilee80:fix/cli-ratio-format-overflow

Conversation

@junkilee80

@junkilee80 junkilee80 commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Problem

An unusually long compression ratio can overflow the 16-byte cratio status buffer after wvunpack successfully verifies or decodes a file. The original reproducer reports a stack-buffer-overflow under ASan.

Change

Apply David Bryant's revised display policy in the wvunpack and wavpack repack status paths: show <-100.00% for ratios at or above 2.0, and use snprintf() with the full buffer size for percentages and bitrates. This branch is rebased onto upstream commit 9bb39ac, which removes the incorrect snprintf to _snprintf macros from the CLI sources.

Verification

  • The original wvunpack -v reproducer reports an ASan stack-buffer-overflow (WRITE of size 17) before the change and exits successfully after it.
  • Clang 21 ASan/UBSan and optimized Linux builds passed focused verify/decode tests for the reproducer, a .wv/.wvc pair, normal lossless and hybrid lossy files, and wavpack repack and encode round trips. Decoded WAV data matched the baseline.
  • A native Windows CLI build was not run locally.

The issue was found during CodeQL and ASan analysis. An earlier PailGen draft was reviewed; this PR uses David's revised patch. The final code and results were reviewed manually.

@dbry

dbry commented Oct 14, 2026

Copy link
Copy Markdown
Owner

Oh my! It's not Windows mapping snprintf() to _snprintf() (which it wouldn't do), it's macros in wvunpack.c and wvtag.c (line 63 in both). Those are very wrong and dangerous and I just deleted them.

Can you remove the "minus one" that you added in the lossy case and rebase this to a single commit?

Thanks!

@junkilee80
junkilee80 force-pushed the fix/cli-ratio-format-overflow branch from e66ea3f to fe61289 Compare October 15, 2026 04:13
Apply David Bryant's revised ratio display in wavpack and wvunpack. Use snprintf with the full buffer size after upstream removes the incorrect _snprintf aliases.
@junkilee80
junkilee80 force-pushed the fix/cli-ratio-format-overflow branch from fe61289 to 58355de Compare October 15, 2026 04:15
@dbry
dbry merged commit 4836421 into dbry:master Oct 15, 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.

2 participants