Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
mp4read: moovin should not alter caller's g_atom
Browse files Browse the repository at this point in the history
moov blocks contain MP4 file metadata. mp4read.c parses moov blocks
using moovin, which recursively calls parse to process subblocks.  In
order to use the parse function, moovin modifies the value of g_atom,
which is a global, static variable. When moovin returns, the position
of the g_atom pointer has thus changed. For example it might be
pointing to the last element of a trak[], meaning that g_atom++
pushes it out-of-bounds.

the result is a buffer-over-read later at line 767.

fix: moovin should not modify g_atom from the point of view of its
caller. Save g_atom's value at the beginning of moovin calls and reset
it before returning.

fixes knik0#34.
hlef committed Aug 10, 2019

Verified

This commit was signed with the committer’s verified signature.
matthewnessworthy Matthew Nessworthy
1 parent b9862e2 commit 2298d5f
Showing 1 changed file with 14 additions and 7 deletions.
21 changes: 14 additions & 7 deletions frontend/mp4read.c
Original file line number Diff line number Diff line change
@@ -797,7 +797,8 @@ static int moovin(int sizemax)
{
long apos = ftell(g_fin);
uint32_t atomsize;
int err;
creator_t *old_atom = g_atom;
int err, ret;

static creator_t mvhd[] = {
{ATOM_NAME, "mvhd"},
@@ -841,28 +842,34 @@ static int moovin(int sizemax)

g_atom = mvhd;
atomsize = sizemax + apos - ftell(g_fin);
if (parse(&atomsize) < 0)
if (parse(&atomsize) < 0) {
g_atom = old_atom;
return ERR_FAIL;
}

fseek(g_fin, apos, SEEK_SET);

while (1)
{
//fprintf(stderr, "TRAK\n");
g_atom = trak;
atomsize = sizemax + apos - ftell(g_fin);
ret = atomsize = sizemax + apos - ftell(g_fin);
if (atomsize < 8)
break;
//fprintf(stderr, "PARSE(%x)\n", atomsize);
err = parse(&atomsize);
//fprintf(stderr, "SIZE: %x/%x\n", atomsize, sizemax);
if (err >= 0)
return sizemax;
if (err != ERR_UNSUPPORTED)
return err;
break;
if (err != ERR_UNSUPPORTED) {
ret = err;
break;
}
//fprintf(stderr, "UNSUPP\n");
}

return sizemax;
g_atom = old_atom;
return ret;
}


0 comments on commit 2298d5f

Please sign in to comment.