Uninitialized stack data ends up in qmf_state
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 78/100
- Issue type
- Bug
- Clarity
- Clearly specified
- Activity status
- Active
- Tech stack
- c
- Domain
- audio-video-rtc
Research direction
Read mp3d_synth_granule in minimp3.h around the linked lines, focusing on how scratch.syn is populated and copied into qmf_state. Confirm the uninitialized values with a state comparison or Valgrind, then check that zeroing the unwritten region makes saved states deterministic while leaving PCM output unchanged.
Written by the indexing model from the issue text.
Description
Hi,
I'm working on a project that uses minimp3 as a dependency, and I need to be able to save and restore (serialize/deserialize) the decoder state. While writing a test for that, I noticed that the same input gave different saved states from one run to the next. Valgrind tracked it down to this:
In mp3d_synth_granule (here), lins points into scratch.syn, which is a stack buffer. The last mp3d_synth call doesn't write every float in its block, but the copy at the end puts that whole region into qmf_state. That means a few uninitialized stack values get stored in the decoder on every granule (15 floats in mono, 30 in stereo).
They get overwritten before they're ever read, so the PCM output isn't affected and no one would ever notice it normally. It's only a problem when you need code side deterministic behavior, like comparing or hashing decoder states.
I patched my local copy like this:
memcpy(lins, qmf_state, sizeof(float)*15*64);
+ memset(lins + 15*64, 0, sizeof(float)*64*nbands);
for (i = 0; i < nbands; i += 2)
With that, the output is unchanged and the state is identical between runs.
It would be nice to have this fixed upstream. Happy to open a PR if that's easier.
Thanks
- Dominant language
- C
- Stars
- 2k
- Forks
- 271
- PR merge metrics
- No merged PRs in 30d
Getting set up
This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from lieff/minimp3
-
[Bug]signed left-shift overflow in mp3dec_skip_id3v1 when parsing the APEv2 tag-size field (CWE-190)Possibly taken A pull request linked to this issue is open or already merged. Open
Difficulty 1/5 Under an hour Newbie friendliness 85/100
-
Integer overflow in Xing/Info VBR header parsing leads to ~10TB allocation (DoS)Possibly taken A pull request linked to this issue is open or already merged. Open
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
-
Difficulty 2/5 Half a day Newbie friendliness 30/100
-
Difficulty 3/5 1-2 days Newbie friendliness 50/100
-
Difficulty 4/5 3-5 days Newbie friendliness 40/100
Similar issues
-
vim.filetype.match() prefixes a URL buffer name with the cwd before passing it to filetype functionsOpen
Difficulty 2/5 1-3 hours Newbie friendliness 73/100
Maintainers usually reply within 1 day
-
stream_curl: retry_count never resets after a successful reconnect, so long sequential streams die after 5 dropsPossibly taken A pull request linked to this issue is open or already merged. Open
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
mpv-player/mpv#18728 ·
Maintainers usually reply within 1 day
-
followup Prio 📌
Difficulty 2/5 1-3 hours Newbie friendliness 66/100
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
cyrusimap/cyrus-imapd#6640 ·
Maintainers usually reply within 2 days
-
Difficulty 2/5 1-3 hours Newbie friendliness 60/100
Maintainers usually reply within 1 day