closes https://github.com/assimp/assimp/issues/1729: check for bit flip when unsigned int overflow happens in x-file parsing.

code/XFileParser.cpp

@@ -87,59 +87,60 @@ static void  dummy_free  (void* /*opaque*/, void* address)  {
 // ------------------------------------------------------------------------------------------------
 // Constructor. Creates a data structure out of the XFile given in the memory block.
 XFileParser::XFileParser( const std::vector<char>& pBuffer)
-{
-    mMajorVersion = mMinorVersion = 0;
-    mIsBinaryFormat = false;
-    mBinaryNumCount = 0;
-    P = End = NULL;
-    mLineNumber = 0;
-    mScene = NULL;
-
+: mMajorVersion( 0 )
+, mMinorVersion( 0 )
+, mIsBinaryFormat( false )
+, mBinaryNumCount( 0 )
+, mP( nullptr )
+, mEnd( nullptr )
+, mLineNumber( 0 )
+, mScene( nullptr ) {
     // vector to store uncompressed file for INFLATE'd X files
     std::vector<char> uncompressed;
 
     // set up memory pointers
-    P = &pBuffer.front();
-    End = P + pBuffer.size() - 1;
+    mP = &pBuffer.front();
+    mEnd = mP + pBuffer.size() - 1;
 
     // check header
-    if( strncmp( P, "xof ", 4) != 0)
-        throw DeadlyImportError( "Header mismatch, file is not an XFile.");
+    if ( 0 != strncmp( mP, "xof ", 4 ) ) {
+        throw DeadlyImportError( "Header mismatch, file is not an XFile." );
+    }
 
     // read version. It comes in a four byte format such as "0302"
-    mMajorVersion = (unsigned int)(P[4] - 48) * 10 + (unsigned int)(P[5] - 48);
-    mMinorVersion = (unsigned int)(P[6] - 48) * 10 + (unsigned int)(P[7] - 48);
+    mMajorVersion = (unsigned int)(mP[4] - 48) * 10 + (unsigned int)(mP[5] - 48);
+    mMinorVersion = (unsigned int)(mP[6] - 48) * 10 + (unsigned int)(mP[7] - 48);
 
     bool compressed = false;
 
     // txt - pure ASCII text format
-    if( strncmp( P + 8, "txt ", 4) == 0)
+    if( strncmp( mP + 8, "txt ", 4) == 0)
         mIsBinaryFormat = false;
 
     // bin - Binary format
-    else if( strncmp( P + 8, "bin ", 4) == 0)
+    else if( strncmp( mP + 8, "bin ", 4) == 0)
         mIsBinaryFormat = true;
 
     // tzip - Inflate compressed text format
-    else if( strncmp( P + 8, "tzip", 4) == 0)
+    else if( strncmp( mP + 8, "tzip", 4) == 0)
     {
         mIsBinaryFormat = false;
         compressed = true;
     }
     // bzip - Inflate compressed binary format
-    else if( strncmp( P + 8, "bzip", 4) == 0)
+    else if( strncmp( mP + 8, "bzip", 4) == 0)
     {
         mIsBinaryFormat = true;
         compressed = true;
     }
     else ThrowException( format() << "Unsupported xfile format '" <<
-       P[8] << P[9] << P[10] << P[11] << "'");
+       mP[8] << mP[9] << mP[10] << mP[11] << "'");
 
     // float size
-    mBinaryFloatSize = (unsigned int)(P[12] - 48) * 1000
-        + (unsigned int)(P[13] - 48) * 100
-        + (unsigned int)(P[14] - 48) * 10
-        + (unsigned int)(P[15] - 48);
+    mBinaryFloatSize = (unsigned int)(mP[12] - 48) * 1000
+        + (unsigned int)(mP[13] - 48) * 100
+        + (unsigned int)(mP[14] - 48) * 10
+        + (unsigned int)(mP[15] - 48);
 
     if( mBinaryFloatSize != 32 && mBinaryFloatSize != 64)
         ThrowException( format() << "Unknown float size " << mBinaryFloatSize << " specified in xfile header." );
@@ -147,7 +148,7 @@ XFileParser::XFileParser( const std::vector<char>& pBuffer)
     // The x format specifies size in bits, but we work in bytes
     mBinaryFloatSize /= 8;
 
-    P += 16;
+    mP += 16;
 
     // If this is a compressed X file, apply the inflate algorithm to it
     if (compressed)
@@ -186,13 +187,13 @@ XFileParser::XFileParser( const std::vector<char>& pBuffer)
         ::inflateInit2(&stream, -MAX_WBITS);
 
         // skip unknown data (checksum, flags?)
-        P += 6;
+        mP += 6;
 
         // First find out how much storage we'll need. Count sections.
-        const char* P1       = P;
+        const char* P1       = mP;
         unsigned int est_out = 0;
 
-        while (P1 + 3 < End)
+        while (P1 + 3 < mEnd)
         {
             // read next offset
             uint16_t ofs = *((uint16_t*)P1);
@@ -216,18 +217,18 @@ XFileParser::XFileParser( const std::vector<char>& pBuffer)
         // Allocate storage and terminating zero and do the actual uncompressing
         uncompressed.resize(est_out + 1);
         char* out = &uncompressed.front();
-        while (P + 3 < End)
+        while (mP + 3 < mEnd)
         {
-            uint16_t ofs = *((uint16_t*)P);
+            uint16_t ofs = *((uint16_t*)mP);
             AI_SWAP2(ofs);
-            P += 4;
+            mP += 4;
 
-            if (P + ofs > End + 2) {
+            if (mP + ofs > mEnd + 2) {
                 throw DeadlyImportError("X: Unexpected EOF in compressed chunk");
             }
 
             // push data to the stream
-            stream.next_in   = (Bytef*)P;
+            stream.next_in   = (Bytef*)mP;
             stream.avail_in  = ofs;
             stream.next_out  = (Bytef*)out;
             stream.avail_out = MSZIP_BLOCK;
@@ -242,15 +243,15 @@ XFileParser::XFileParser( const std::vector<char>& pBuffer)
 
             // and advance to the next offset
             out +=  MSZIP_BLOCK - stream.avail_out;
-            P   += ofs;
+            mP   += ofs;
         }
 
         // terminate zlib
         ::inflateEnd(&stream);
 
         // ok, update pointers to point to the uncompressed file data
-        P = &uncompressed[0];
-        End = out;
+        mP = &uncompressed[0];
+        mEnd = out;
 
         // FIXME: we don't need the compressed data anymore, could release
         // it already for better memory usage. Consider breaking const-co.
@@ -647,8 +648,8 @@ void XFileParser::ParseDataObjectMeshVertexColors( Mesh* pMesh)
         if( !mIsBinaryFormat)
         {
             FindNextNoneWhiteSpace();
-            if( *P == ';' || *P == ',')
-                P++;
+            if( *mP == ';' || *mP == ',')
+                mP++;
         }
     }
 
@@ -678,8 +679,8 @@ void XFileParser::ParseDataObjectMeshMaterialList( Mesh* pMesh)
     // commented out version check, as version 03.03 exported from blender also has 2 semicolons
     if( !mIsBinaryFormat) // && MajorVersion == 3 && MinorVersion <= 2)
     {
-        if(P < End && *P == ';')
-            ++P;
+        if(mP < mEnd && *mP == ';')
+            ++mP;
     }
 
     // if there was only a single material index, replicate it on all faces
@@ -1029,12 +1030,12 @@ void XFileParser::TestForSeparator()
     return;
 
   FindNextNoneWhiteSpace();
-  if( P >= End)
+  if( mP >= mEnd)
     return;
 
   // test and skip
-  if( *P == ';' || *P == ',')
-    P++;
+  if( *mP == ';' || *mP == ',')
+    mP++;
 }
 
 // ------------------------------------------------------------------------------------------------
@@ -1062,46 +1063,54 @@ std::string XFileParser::GetNextToken()
         // in binary mode it will only return NAME and STRING token
         // and (correctly) skip over other tokens.
 
-        if( End - P < 2) return s;
+        if( mEnd - mP < 2) return s;
         unsigned int tok = ReadBinWord();
         unsigned int len;
 
         // standalone tokens
         switch( tok)
         {
-            case 1:
-                // name token
-                if( End - P < 4) return s;
-                len = ReadBinDWord();
-                if( End - P < int(len)) return s;
-                s = std::string(P, len);
-                P += len;
+            case 1: {
+                    // name token
+                    if ( mEnd - mP < 4 ) return s;
+                    len = ReadBinDWord();
+                    const int bounds( mEnd - mP );
+                    const int iLen( len );
+                    if ( iLen < 0 ) {
+                        return s;
+                    }
+                    if ( bounds < iLen ) {
+                        return s;
+                    }
+                    s = std::string( mP, len );
+                    mP += len;
+                }
                 return s;
             case 2:
                 // string token
-                if( End - P < 4) return s;
+                if( mEnd - mP < 4) return s;
                 len = ReadBinDWord();
-                if( End - P < int(len)) return s;
-                s = std::string(P, len);
-                P += (len + 2);
+                if( mEnd - mP < int(len)) return s;
+                s = std::string(mP, len);
+                mP += (len + 2);
                 return s;
             case 3:
                 // integer token
-                P += 4;
+                mP += 4;
                 return "<integer>";
             case 5:
                 // GUID token
-                P += 16;
+                mP += 16;
                 return "<guid>";
             case 6:
-                if( End - P < 4) return s;
+                if( mEnd - mP < 4) return s;
                 len = ReadBinDWord();
-                P += (len * 4);
+                mP += (len * 4);
                 return "<int_list>";
             case 7:
-                if( End - P < 4) return s;
+                if( mEnd - mP < 4) return s;
                 len = ReadBinDWord();
-                P += (len * mBinaryFloatSize);
+                mP += (len * mBinaryFloatSize);
                 return "<flt_list>";
             case 0x0a:
                 return "{";
@@ -1159,19 +1168,19 @@ std::string XFileParser::GetNextToken()
     else
     {
         FindNextNoneWhiteSpace();
-        if( P >= End)
+        if( mP >= mEnd)
             return s;
 
-        while( (P < End) && !isspace( (unsigned char) *P))
+        while( (mP < mEnd) && !isspace( (unsigned char) *mP))
         {
             // either keep token delimiters when already holding a token, or return if first valid char
-            if( *P == ';' || *P == '}' || *P == '{' || *P == ',')
+            if( *mP == ';' || *mP == '}' || *mP == '{' || *mP == ',')
             {
                 if( !s.size())
-                    s.append( P++, 1);
+                    s.append( mP++, 1);
                 break; // stop for delimiter
             }
-            s.append( P++, 1);
+            s.append( mP++, 1);
         }
     }
     return s;
@@ -1186,18 +1195,18 @@ void XFileParser::FindNextNoneWhiteSpace()
     bool running = true;
     while( running )
     {
-        while( P < End && isspace( (unsigned char) *P))
+        while( mP < mEnd && isspace( (unsigned char) *mP))
         {
-            if( *P == '\n')
+            if( *mP == '\n')
                 mLineNumber++;
-            ++P;
+            ++mP;
         }
 
-        if( P >= End)
+        if( mP >= mEnd)
             return;
 
         // check if this is a comment
-        if( (P[0] == '/' && P[1] == '/') || P[0] == '#')
+        if( (mP[0] == '/' && mP[1] == '/') || mP[0] == '#')
             ReadUntilEndOfLine();
         else
             break;
@@ -1214,22 +1223,22 @@ void XFileParser::GetNextTokenAsString( std::string& poString)
     }
 
     FindNextNoneWhiteSpace();
-    if( P >= End)
+    if( mP >= mEnd)
         ThrowException( "Unexpected end of file while parsing string");
 
-    if( *P != '"')
+    if( *mP != '"')
         ThrowException( "Expected quotation mark.");
-    ++P;
+    ++mP;
 
-    while( P < End && *P != '"')
-        poString.append( P++, 1);
+    while( mP < mEnd && *mP != '"')
+        poString.append( mP++, 1);
 
-    if( P >= End-1)
+    if( mP >= mEnd-1)
         ThrowException( "Unexpected end of file while parsing string");
 
-    if( P[1] != ';' || P[0] != '"')
+    if( mP[1] != ';' || mP[0] != '"')
         ThrowException( "Expected quotation mark and semicolon at the end of a string.");
-    P+=2;
+    mP+=2;
 }
 
 // ------------------------------------------------------------------------------------------------
@@ -1238,35 +1247,35 @@ void XFileParser::ReadUntilEndOfLine()
     if( mIsBinaryFormat)
         return;
 
-    while( P < End)
+    while( mP < mEnd)
     {
-        if( *P == '\n' || *P == '\r')
+        if( *mP == '\n' || *mP == '\r')
         {
-            ++P; mLineNumber++;
+            ++mP; mLineNumber++;
             return;
         }
 
-        ++P;
+        ++mP;
     }
 }
 
 // ------------------------------------------------------------------------------------------------
 unsigned short XFileParser::ReadBinWord()
 {
-    ai_assert(End - P >= 2);
-    const unsigned char* q = (const unsigned char*) P;
+    ai_assert(mEnd - mP >= 2);
+    const unsigned char* q = (const unsigned char*) mP;
     unsigned short tmp = q[0] | (q[1] << 8);
-    P += 2;
+    mP += 2;
     return tmp;
 }
 
 // ------------------------------------------------------------------------------------------------
-unsigned int XFileParser::ReadBinDWord()
-{
-    ai_assert(End - P >= 4);
-    const unsigned char* q = (const unsigned char*) P;
+unsigned int XFileParser::ReadBinDWord() {
+    ai_assert(mEnd - mP >= 4);
+
+    const unsigned char* q = (const unsigned char*) mP;
     unsigned int tmp = q[0] | (q[1] << 8) | (q[2] << 16) | (q[3] << 24);
-    P += 4;
+    mP += 4;
     return tmp;
 }
 
@@ -1275,20 +1284,20 @@ unsigned int XFileParser::ReadInt()
 {
     if( mIsBinaryFormat)
     {
-        if( mBinaryNumCount == 0 && End - P >= 2)
+        if( mBinaryNumCount == 0 && mEnd - mP >= 2)
         {
             unsigned short tmp = ReadBinWord(); // 0x06 or 0x03
-            if( tmp == 0x06 && End - P >= 4) // array of ints follows
+            if( tmp == 0x06 && mEnd - mP >= 4) // array of ints follows
                 mBinaryNumCount = ReadBinDWord();
             else // single int follows
                 mBinaryNumCount = 1;
         }
 
         --mBinaryNumCount;
-        if ( End - P >= 4) {
+        if ( mEnd - mP >= 4) {
             return ReadBinDWord();
         } else {
-            P = End;
+            mP = mEnd;
             return 0;
         }
     } else
@@ -1299,24 +1308,24 @@ unsigned int XFileParser::ReadInt()
 
         // check preceding minus sign
         bool isNegative = false;
-        if( *P == '-')
+        if( *mP == '-')
         {
             isNegative = true;
-            P++;
+            mP++;
         }
 
         // at least one digit expected
-        if( !isdigit( *P))
+        if( !isdigit( *mP))
             ThrowException( "Number expected.");
 
         // read digits
         unsigned int number = 0;
-        while( P < End)
+        while( mP < mEnd)
         {
-            if( !isdigit( *P))
+            if( !isdigit( *mP))
                 break;
-            number = number * 10 + (*P - 48);
-            P++;
+            number = number * 10 + (*mP - 48);
+            mP++;
         }
 
         CheckForSeparator();
@@ -1329,10 +1338,10 @@ ai_real XFileParser::ReadFloat()
 {
     if( mIsBinaryFormat)
     {
-        if( mBinaryNumCount == 0 && End - P >= 2)
+        if( mBinaryNumCount == 0 && mEnd - mP >= 2)
         {
             unsigned short tmp = ReadBinWord(); // 0x07 or 0x42
-            if( tmp == 0x07 && End - P >= 4) // array of floats following
+            if( tmp == 0x07 && mEnd - mP >= 4) // array of floats following
                 mBinaryNumCount = ReadBinDWord();
             else // single float following
                 mBinaryNumCount = 1;
@@ -1341,22 +1350,22 @@ ai_real XFileParser::ReadFloat()
         --mBinaryNumCount;
         if( mBinaryFloatSize == 8)
         {
-            if( End - P >= 8) {
-                ai_real result = (ai_real) (*(double*) P);
-                P += 8;
+            if( mEnd - mP >= 8) {
+                ai_real result = (ai_real) (*(double*) mP);
+                mP += 8;
                 return result;
             } else {
-                P = End;
+                mP = mEnd;
                 return 0;
             }
         } else
         {
-            if( End - P >= 4) {
-                ai_real result = *(ai_real*) P;
-                P += 4;
+            if( mEnd - mP >= 4) {
+                ai_real result = *(ai_real*) mP;
+                mP += 4;
                 return result;
             } else {
-                P = End;
+                mP = mEnd;
                 return 0;
             }
         }
@@ -1367,21 +1376,21 @@ ai_real XFileParser::ReadFloat()
     // check for various special strings to allow reading files from faulty exporters
     // I mean you, Blender!
     // Reading is safe because of the terminating zero
-    if( strncmp( P, "-1.#IND00", 9) == 0 || strncmp( P, "1.#IND00", 8) == 0)
+    if( strncmp( mP, "-1.#IND00", 9) == 0 || strncmp( mP, "1.#IND00", 8) == 0)
     {
-        P += 9;
+        mP += 9;
         CheckForSeparator();
         return 0.0;
     } else
-    if( strncmp( P, "1.#QNAN0", 8) == 0)
+    if( strncmp( mP, "1.#QNAN0", 8) == 0)
     {
-        P += 8;
+        mP += 8;
         CheckForSeparator();
         return 0.0;
     }
 
     ai_real result = 0.0;
-    P = fast_atoreal_move<ai_real>( P, result);
+    mP = fast_atoreal_move<ai_real>( mP, result);
 
     CheckForSeparator();
 

code/XFileParser.h

@@ -49,10 +49,8 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #include <assimp/types.h>
 
-namespace Assimp
-{
-    namespace XFile
-    {
+namespace Assimp {
+    namespace XFile {
         struct Node;
         struct Mesh;
         struct Scene;
@@ -61,21 +59,20 @@ namespace Assimp
         struct AnimBone;
     }
 
-/** The XFileParser reads a XFile either in text or binary form and builds a temporary
- * data structure out of it.
- */
-class XFileParser
-{
+/**
+  *     @brief  The XFileParser reads a XFile either in text or binary form and builds a temporary
+  *             data structure out of it.
+  */
+class XFileParser {
 public:
-    /** Constructor. Creates a data structure out of the XFile given in the memory block.
-     * @param pBuffer Null-terminated memory buffer containing the XFile
-     */
+    /// Constructor. Creates a data structure out of the XFile given in the memory block.
+    /// @param pBuffer Null-terminated memory buffer containing the XFile
     explicit XFileParser( const std::vector<char>& pBuffer);
 
-    /** Destructor. Destroys all imported data along with it */
+    /// Destructor. Destroys all imported data along with it
     ~XFileParser();
 
-    /** Returns the temporary representation of the imported data */
+    /// Returns the temporary representation of the imported data.
     XFile::Scene* GetImportedData() const { return mScene; }
 
 protected:
@@ -101,10 +98,10 @@ protected:
     //! places pointer to next begin of a token, and ignores comments
     void FindNextNoneWhiteSpace();
 
-    //! returns next parseable token. Returns empty string if no token there
+    //! returns next valid token. Returns empty string if no token there
     std::string GetNextToken();
 
-    //! reads header of dataobject including the opening brace.
+    //! reads header of data object including the opening brace.
     //! returns false if error happened, and writes name of object
     //! if there is one
     void readHeadOfDataObject( std::string* poName = NULL);
@@ -118,8 +115,8 @@ protected:
     //! checks for a separator char, either a ',' or a ';'
     void CheckForSeparator();
 
-  /// tests and possibly consumes a separator char, but does nothing if there was no separator
-  void TestForSeparator();
+    /// tests and possibly consumes a separator char, but does nothing if there was no separator
+    void TestForSeparator();
 
     //! reads a x file style string
     void GetNextTokenAsString( std::string& poString);
@@ -138,27 +135,23 @@ protected:
     /** Throws an exception with a line number and the given text. */
     AI_WONT_RETURN void ThrowException( const std::string& pText) AI_WONT_RETURN_SUFFIX;
 
-    /** Filters the imported hierarchy for some degenerated cases that some exporters produce.
-     * @param pData The sub-hierarchy to filter
-     */
+    /**
+      * @brief  Filters the imported hierarchy for some degenerated cases that some exporters produce.
+      * @param pData The sub-hierarchy to filter
+      */
     void FilterHierarchy( XFile::Node* pNode);
 
 protected:
     unsigned int mMajorVersion, mMinorVersion; ///< version numbers
     bool mIsBinaryFormat; ///< true if the file is in binary, false if it's in text form
     unsigned int mBinaryFloatSize; ///< float size in bytes, either 4 or 8
-    // counter for number arrays in binary format
-    unsigned int mBinaryNumCount;
-
-    const char* P;
-    const char* End;
-
-    /// Line number when reading in text format
-    unsigned int mLineNumber;
-
-    /// Imported data
-    XFile::Scene* mScene;
+    unsigned int mBinaryNumCount; /// < counter for number arrays in binary format
+    const char* mP;
+    const char* mEnd;
+    unsigned int mLineNumber; ///< Line number when reading in text format
+    XFile::Scene* mScene; ///< Imported data
 };
 
-}
+} //! ns Assimp
+
 #endif // AI_XFILEPARSER_H_INC