pali / udftools

Linux tools for UDF filesystems and DVD/CD-R(W) drives
GNU General Public License v2.0
108 stars 30 forks source link

wrudf.c: potential NULL dereference in initialize() #51

Closed i386x closed 3 years ago

i386x commented 3 years ago

In function initialize(), please look at this code snippet (starting at line 158):

/* read Volume Descriptor Sequence */
blkno = extentMainVolDescSeq.extLocation;
len = extentMainVolDescSeq.extLength;

for( i = 0; i < len; blkno++, i += 2048 ) {
    int inMainSeq = 1;

    if( (p = readTaggedBlock(blkno, ABSOLUTE)) == NULL ) {
        if( !inMainSeq )
            fail("Volume Descriptor Sequences read failure\n");
        blkno = extentRsrvVolDescSeq.extLocation;
        len = extentRsrvVolDescSeq.extLength;
        inMainSeq = 0;
        i = 0;
    }
    switch( p->descTag.tagIdent ) {
    ...

When readTaggedBlock() returns NULL, !inMainSeq evaluates to false and p in switch statement will have the value NULL. I am not a UDF expert, but shouldn't the code above look something like this?

/* read Volume Descriptor Sequence */
blkno = extentMainVolDescSeq.extLocation;
len = extentMainVolDescSeq.extLength;
int inMainSeq = 1;

for( i = 0; i < len; blkno++, i += 2048 ) {
    if( (p = readTaggedBlock(blkno, ABSOLUTE)) == NULL ) {
        if( !inMainSeq )
            fail("Volume Descriptor Sequences read failure\n");
        blkno = extentRsrvVolDescSeq.extLocation;
        len = extentRsrvVolDescSeq.extLength;
        inMainSeq = 0;
        i = (uint32_t) -2048;
        continue;
    }
    switch( p->descTag.tagIdent ) {
    ...
pali commented 3 years ago

You are right, this is an issue and I have fixed it now in commit 7a432b0bcd76cbd6a79925cde213f5d9073df137. See that also blkno was incorrectly calculated (due to autoincrement in for-loop). Anyway, wrudf is currently unmaintained and it is possible that there are more similar failures and crashes. If you are interested in fixing wrudf, fell free to provide patches.

i386x commented 3 years ago

Thanks for the fast resolution!