Infinite loop trying to parse corrupted mp4

Issue #218 resolved
Silvano Riz created an issue

After the last merge that fixes the issue #216, a potential infinite loop has been introduced in the Mp4BoxHeader#seekWithinLevel(FileChannel, String)

From my initial tests, the infinite loop is happening if the file is corrupted. Anyway, this is dangerous and the parser should identify that the file cannot be parsed and throw an error instead.

Attached there is the file to replicate the issue. A simple test like the following will re-create the issue

public void testGetAudioFileInfo_corrupted() throws Exception {
    final File file = AbstractTestCase.copyAudioToTmp("corrupt.mp4");
    final Mp4FileReader mp4FileReader = new Mp4FileReader();
    final AudioFile audioFile = mp4FileReader.read(file);
}

Comments (3)

  1. Silvano Riz reporter

    Hi @ijabz, I don't have a fix because I'm not too familiar with the mp4 parsing and structure. As far as I can see I think that there should be some validation against the value returned by data64bitLengthBuffer.getLong() at line 297 of the Mp4BoxHeader.java file. I guess it should be something similar to what's defined for the non 64bit data length logic.

    it might be something like the following, but I'm not sure:

            //64bit data length
            if(boxHeader.getLength() == 1)
            {
                ByteBuffer data64bitLengthBuffer = ByteBuffer.allocate(DATA_64BITLENGTH);
                data64bitLengthBuffer.order(ByteOrder.BIG_ENDIAN);
                bytesRead = fc.read(data64bitLengthBuffer);
                if (bytesRead != DATA_64BITLENGTH)
                {
                    return null;
                }
                data64bitLengthBuffer.rewind();
                long length = data64bitLengthBuffer.getLong();
                if (length < Mp4BoxHeader.HEADER_LENGTH){
                    return null;
                }
                fc.position(fc.position() + length - REALDATA_64BITLENGTH);
                logger.severe("Skipped 64bit data length, now at:" + fc.position());
            }
    
  2. Log in to comment