realityking / mp4v2

Automatically exported from code.google.com/p/mp4v2
Other
0 stars 0 forks source link

Wrong usage of "isalnum" in "MP4Atom::IsReasonableType" #104

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Open a MP4 file which contains metadata tags written by QuickTime Player 
(those are the "udta" tags which start with "@"). Current version of QuickTime 
Payer for Windows still writes those tags. The problem which is reported here 
is though not related to QuickTime Player, it applies to all "unknown" tags.

What is the expected output? What do you see instead?
The characters of those tags are processed by "MP4Atom::IsReasonableType" and 
feed to "isalnum". However, the characters are not casted to "unsigned char" 
and will therefore lead to unpredictable results of "isalnum" because of the 
"char->int" signed conversion. None of the "isXXX" from "ctype" is allowed to 
get called this way. The debug version of the Visual C++ C-RTL throws an assert 
if "isalnum" is invoked with a value which is outside the specified range. 
Please note this is not a detail of the Microsoft C-RTL, but applies to all 
ANSI-C RTLs, other C-RTL usually don't throw an assert though and may just 
silently give wrong results or even crash.

What version of the product are you using? On what operating system?
Trunk, rev. 468.

Please provide any additional information below.

The correct code would look like this:

bool MP4Atom::IsReasonableType(const char* type)
{
    for (uint8_t i = 0; i < 4; i++) {
        if (isalnum((unsigned char)type[i])) {
            continue;
        }
        if (i == 3 && type[i] == ' ') {
            continue;
        }
        return false;
    }
    return true;
}

Original issue reported on code.google.com by Skaa...@gmail.com on 30 May 2011 at 7:16

GoogleCodeExporter commented 9 years ago
Thanks--I actually was alerted to this earlier, but it looks like I messed up 
the fix.  Should be resolved in r470.

Original comment by kid...@gmail.com on 31 May 2011 at 5:45