Skip to content

fix(media): saturate MP3/JPEG size overflows and free FFmpeg packet on throw - #12199

Merged
oneandonlydean merged 5 commits into
boykopovar:mainfrom
MarcZX10:fix/media-library-overflows
Oct 28, 2026
Merged

oneandonlydean merged 5 commits into
boykopovar:mainfrom
MarcZX10:fix/media-library-overflows

Conversation

@MarcZX10

Copy link
Copy Markdown
Contributor

What

Three independent integer-overflow / leak fixes in media libraries:

  • libSceAjm: ParseMp3Ofl computed frames * samples_per_channel - (delay + padding) in uint32, so a hostile Xing/LAME header could wrap the product or underflow the subtraction. It is now computed in uint64, returns 0 when delay plus padding covers all samples, and clamps to UINT32_MAX. Existing parse results are unchanged.
  • libSceJpegDec: fillImageInfo computed the progressive coefficient size (mcuColumns * mcuRows * blocksPerMcu * 128 + 64) in uint32, so huge dimensions wrapped it, sceJpegDecParseHeader reported a small size and the coefficient_mem_size check in sceJpegDecDecode was bypassed. It is now computed in uint64; if it does not fit in uint32, both functions return 0x80650005, the code sceJpegDecDecode already returns for insufficient coefficient memory (now a named constant).
  • libSceAudiodec: decodePacket held the AVPacket and AVFrame as raw pointers freed only at the end, so the std::runtime_error for an unsupported FFmpeg sample format leaked both. They are now unique_ptrs with av_packet_free / av_frame_free deleters. No test: reaching that branch needs a decoder that emits a format other than FLTP/S16P, which the bundled MP3/AAC decoders do not do.

Each fix is its own commit, followed by its test commit (Ajm and JpegDec).

Not included, on purpose:

  • libSceNgs2 ConsumeSamples (out-of-range llround with a huge pitch): open PR feat(ngs2): implement custom PitchShift and external render jobs #11315 rewrites and moves those lines, so a change here would conflict.
  • libSceAudioOut sceAudioOutOpen length validation: AudioOutBufferSize.cpp asserts that opening with len 0x80000000 / 0x40000000 succeeds and that the first output throws, so rejecting at open would change behaviour an existing test asserts.

Tested

Linux x86_64. Built guest_ajm_tests, guest_jpeg_dec_tests and guest_audiodec_tests, and ran ctest guest_ajm, guest_jpeg_dec, guest_audiodec: 3/3 passed. New cases: a LAME header with a frame count smaller than delay plus padding (expects 0) and one with frame count 0xFFFFFFFF (expects UINT32_MAX); a progressive JPEG patched to 65535x65535 (expects 0x80650005 from sceJpegDecParseHeader). tools/check_conventions.py --base upstream/main and git diff --check pass. The Audiodec change is only reasoned (existing guest_audiodec still passes). The helper's SYNTAX pre-check failed on missing include paths on the host (known limitation); the build itself exited 0.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Fixes: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

ParseMp3Ofl computed frames * samples_per_channel - (delay + padding) in
uint32. A hostile Xing/LAME header could wrap the multiplication or make
the subtraction underflow, reporting a huge bogus total_samples.

Compute in uint64, return 0 when the trimmed amount covers all samples
and clamp to UINT32_MAX otherwise.
Add a header whose frame count is smaller than the delay plus padding
(expects 0) and one with a 0xFFFFFFFF frame count (expects UINT32_MAX).
… overflows

fillImageInfo computed mcuColumns * mcuRows * blocksPerMcu * 128 + 64 in
uint32. A progressive header with huge dimensions wrapped the result, so
sceJpegDecParseHeader reported a small coefficient_mem_size and the
coefficient memory check in sceJpegDecDecode was bypassed.

Compute the size in uint64 and fail with the coefficient memory error
(0x80650005, already returned when the supplied coefficient memory is too
small) when it does not fit in uint32.
A progressive header patched to 65535x65535 must make
sceJpegDecParseHeader return 0x80650005 instead of a wrapped size.
…mat is unsupported

decodePacket allocated the AVPacket and AVFrame as raw pointers and only
released them at the end of the function, so the std::runtime_error thrown
for an unconverted sample format leaked both.

Hold them in unique_ptr with av_packet_free/av_frame_free deleters so
every exit path releases them. Behaviour is otherwise unchanged.
@github-actions

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
PR 9760 core/libs/prx/libSceJpegDec/Export.cpp L181-224

Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.

Pull request Shared files not compared
PR 3205 core/libs/tests/GuestAjm.cpp

@github-actions github-actions Bot added the prx label Oct 28, 2026
@JJenkx

JJenkx commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

A Windows run, since the description lists none. Head 6ddccfe33b was merged onto main 82d5f01f28 with no conflicts. Setup: Windows 11 Enterprise 10.0.22621, WinLibs GCC 15.2.0 UCRT r7, Ninja, Release, BUILD_TESTING=ON.

libs builds, and guest_ajm and guest_jpeg_dec pass in each of 3 runs.

AI-assisted: yes (Claude Code).

@maskym

maskym commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Windows result for head 6ddccfe33b, merged into main 87b810a1db (the PR's Tested section names no Windows run).

Host: Windows 11 26300, Intel i7-12700K, WinLibs GCC 15.2.0 (x86_64-ucrt-posix-seh, r6), Release, BUILD_TESTING=ON, Ninja. Build clean.

The tree also carried the open PRs #12290, #12288, #12285, #12269, #12265, #12232, #12206, #12201, #12197, #12165, #12146, all merged cleanly, so the run is a joint one; the tests of this PR are guest_ajm (0.09 s), guest_ajm_aac_multichannel (0.06 s), guest_ajm_teardown (? s), guest_ajm_vibration (0.05 s), guest_ajm_wvorbis (0.04 s), guest_jpeg_dec (0.04 s), all passed (every test registered from the tree's changed test files: 38/38). Not run on a PS5.

AI-assisted: yes (Claude Code), the runs and the numbers above are from this machine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants