Skip to content

Vulnerability fixes#3

Open
dchengTSC wants to merge 17 commits into
mainfrom
techsmith/vulnerability-fixes
Open

Vulnerability fixes#3
dchengTSC wants to merge 17 commits into
mainfrom
techsmith/vulnerability-fixes

Conversation

@dchengTSC

Copy link
Copy Markdown
Collaborator

No description provided.

Add support for reading VP9 video tracks in MP4 files:
- Add MP4Vp09Atom class that properly parses the vp09 sample entry,
  including its child atoms (vpcC, btrt, colr, pasp)
- Register vp09 in the atom factory and as an expected stsd child

Add MP4GetTrackAtomData/MP4FreeTrackAtomData API:
- Allows reading the raw data of any track atom by path
- Useful for extracting codec configuration boxes (e.g. vpcC, avcC)
  that don't have dedicated accessor functions
- Memory is allocated by the library and freed via MP4FreeTrackAtomData
  to avoid CRT mismatch issues on Windows
Add support for creating and reading TSC2 video tracks in MP4 files:
- Add MP4Tsc2Atom class that defines the tsc2 sample entry structure
  (dataReferenceIndex, width, height, compressorName, esds child atom)
- Register tsc2 in the atom factory and as an expected stsd child
- Add MP4File::AddTSC2VideoTrack() method for creating TSC2 tracks
- Add MP4AddTSC2VideoTrack() public C API

TSC2 is TechSmith's proprietary screen capture codec used in TREC
(TechSmith Recording) container files which are MP4-based.
Add an optional callback parameter to MP4Read() that allows callers
to control which atoms are fully parsed during file reading. When
the callback returns false for a given atom type, the atom's
properties and children are skipped (the atom is still recorded in
the tree but its content is not read).

This is useful for error recovery: if a file fails to parse due to
a malformed atom (e.g. corrupted udta or text atoms), the caller
can retry with a callback that skips problematic atoms, allowing
the rest of the file structure to be read successfully.

API changes:
- Add MP4ShouldParseAtomCallback typedef in file.h
- MP4Read() gains an optional second parameter (default NULL)
- MP4File stores and exposes the callback via get/set methods
- MP4Atom::Read() checks the callback before parsing
Add MP4PNGAtom class for properly parsing 'png ' sample entry atoms
found in QuickTime MOV files that use PNG-encoded video frames.

Without this registration, PNG atoms are treated as unknown and their
width/height properties are not directly accessible (requiring
fallback to tkhd dimensions). With this change, the stsd.png .width
and stsd.png .height properties are properly parsed.

Changes:
- Add MP4PNGAtom class in atoms.h with standard video sample entry
  layout (dataReferenceIndex, width, height, compressorName)
- Register 'png ' in the atom factory
- Add as expected stsd child atom
- esds child is optional (unlike TSC2) since PNG MOVs may not have it
Add missing C API functions needed by consuming code:

MP4GetSampleFileOffset():
- Returns the byte offset within the file of a given sample
- The underlying MP4Track::GetSampleFileOffset() already existed but
  was protected; moved to public and added MP4File wrapper + C API
- Used by TRECBackupWriter and TRECMP4TrackReader

MP4FreeTrackName():
- Frees memory allocated by MP4GetTrackName()
- Needed on Windows where CRT heap mismatch between DLL and client
  can cause crashes if client calls free() directly
- Wrapper around MP4Free()
so we don't end up with library names like libmp4v2.2.dylib
@dchengTSC dchengTSC force-pushed the techsmith/vulnerability-fixes branch from c4b39ba to 1ed8a87 Compare June 11, 2026 21:24
Several atoms in atom_meta.cpp and atom_sdtp.cpp compute a variable-length
property size by subtracting a fixed constant from m_size (the atom's content
size read from the file). None of these subtractions validated that m_size
was large enough, allowing an integer underflow.

How the vulnerability works:

  1. Attacker crafts an MP4 with a 'data' atom whose total size is 10 bytes
     (8-byte header + 2 bytes of content), giving m_size = 2.

  2. MP4DataAtom::Read() computes: metadata.SetValueSize(m_size - 8)
     Since m_size is uint64_t, the subtraction 2 - 8 wraps to
     0xFFFFFFFFFFFFFFFA.

  3. SetValueSize truncates this to uint32_t: 0xFFFFFFFA (~4 GB).

  4. MP4Malloc(0xFFFFFFFA) is called. On systems with memory overcommit
     (e.g. Linux default), this succeeds, and the subsequent ReadBytes
     writes file data (and eventually uninitialized memory) into a ~4 GB
     heap buffer -- a classic heap buffer overflow that can be exploited
     for arbitrary code execution.

  5. On systems without overcommit (e.g. macOS), malloc fails and the
     library crashes (denial of service).

The same pattern exists in MP4ItmfHdlrAtom (m_size - 24),
MP4MeanAtom (m_size - 4), MP4NameAtom (m_size - 4), and
MP4SdtpAtom (m_size - 4).

Fix: Add bounds checks before each subtraction, throwing an exception
if m_size is too small to contain the required fixed-size properties.

Includes a POC generator (test/poc_cve_meta_underflow.cpp), the generated
malformed MP4 (test/poc_meta_underflow.mp4, 107 bytes), and a test harness
(test/test_poc_meta_underflow.cpp) that verifies the fix rejects the file.
GetSampleSize() and GetMaxSampleSize() multiply attacker-controlled
uint32_t values (fixedSampleSize from stsz, or per-sample sizes) by
m_bytesPerSample (derived from audio channel count and bit depth)
without overflow checking. A crafted file can set stsz.sampleSize to
a large value (e.g. 0x40000001) combined with a multi-channel audio
track (m_bytesPerSample=4), causing the uint32 product to wrap to a
small value. This undersized value is then used as an allocation size
in ReadSample, followed by a read of the true (large) byte count,
resulting in a heap buffer overflow.

Fix: all multiplications by m_bytesPerSample in GetSampleSize() and
GetMaxSampleSize() now widen to uint64_t and throw an exception if
the result exceeds UINT32_MAX.

Also fix test harness to resolve POC file paths via POC_TEST_FILES_DIR
so tests pass regardless of working directory.
In GetSampleFileOffset(), accumulated sample sizes were stored in a
uint32_t which wraps on overflow. With attacker-controlled sample sizes
(e.g. 3 samples of 0x80000000 each), summing them wraps to 0, producing
an arbitrary file seek position. Combined with stco/co64 chunk offsets,
this enables out-of-bounds reads from wrong file positions.

Fix:
- Change sampleOffset and m_cachedSfoSampleOffset to uint64_t to prevent
  the accumulation from wrapping.
- Validate the final computed file offset (chunkOffset + sampleOffset)
  against the actual file size before returning.
- Fix Vuln enzo1982#4 table validation to skip implicit properties when computing
  minimum entry size (was false-positive rejecting valid stsc atoms).

POC: test/poc_offset_overflow.mp4 - video track with 3 samples of size
0x80000000 in one chunk; reading sample 3 accumulates 0x100000000 which
correctly exceeds file size and throws.
MP4Realloc accepted a uint32_t size parameter, causing silent truncation
when callers passed uint64_t or size_t values exceeding 4GB. This allowed
a crafted file to trigger an undersized allocation followed by a buffer
overflow write. For example, mp4file_io.cpp passed m_memoryBufferSize
(uint64_t) directly, and mp4array.h passed newSize * sizeof(type) which
could exceed 32 bits.

Fix:
- Change MP4Realloc parameter from uint32_t to size_t
- Add overflow-checked size_t multiplication in MP4Array::Insert() and
  MP4Array::Resize() before calling MP4Realloc
- Add SIZE_MAX guard in mp4file_io.cpp WriteBytes memory buffer path
- Use explicit size_t in mp4track.cpp chunk buffer reallocation

POC: test/poc_realloc_truncation.mp4 - stts atom with entryCount=0x20000001
and 8-byte entries requires 0x100000008 bytes, which truncated to 8 bytes
in the old uint32_t parameter.
ReadSample() used GetSampleSize() directly as an allocation size with no
upper-bound validation. A malicious file can set stsz sample sizes up to
0xFFFFFFFF (~4GB), causing an enormous heap allocation from a tiny file
(OOM denial-of-service).

Added a check that rejects sample sizes exceeding the actual file size
before any allocation occurs. Includes POC test file and test case.
ReadSample validated sampleSize and fileOffset individually against
the file size, but never checked that fileOffset + sampleSize stays
within bounds. An attacker can craft a file where the stco chunk offset
points near the end of the file and stsz declares a sample size that is
smaller than the file, but together they cause a read past EOF. This
enables out-of-bounds reads from the file buffer.

Fix: validate fileOffset + sampleSize <= fileSize before seeking and
reading. Add POC test file and test case.
@dchengTSC dchengTSC force-pushed the techsmith/vulnerability-fixes branch from bec2437 to 0a54710 Compare June 11, 2026 21:35
…allocation

stsc: The multiplication (firstChunk[i+1] - firstChunk[i]) * samplesPerChunk[i]
in MP4StscAtom::Read() uses attacker-controlled uint32 values whose product can
overflow, corrupting the firstSample lookup table and causing out-of-bounds array
accesses downstream. Fixed by using uint64_t intermediate arithmetic with overflow
detection before accumulating into sampleId.

trun: MP4TrunAtom::Read() reads sampleCount from file metadata and allocates
per-sample table entries based on flags (up to 16 bytes/entry). A malicious file
using an extended-size atom header can bypass the existing table entry count
validation (which checks against declared atom size). Fixed by additionally
validating that sampleCount * bytesPerEntry does not exceed the actual remaining
file size.

Added POC test files and test cases for both fixes.

@KMojek KMojek left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to include the commits from PRs #1 and #2 here? Either way, looks good to me! 👍

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