Skip to content

Commit

Permalink
Handle unknown top level boxes with size == 0
Browse files Browse the repository at this point in the history
In avifParse() right now we skip over unknown top level boxes with
size == 0 incorrectly.

The correct behavior is to simply fail on them because if we reach
that point it means that:
1) there are no more boxes left to parse (since the current unknown
   box goes on until the end of stream).
2) we haven't found all the necessary boxes for the parsing to be
   considered a succeess (either ftyp or moov or meta was not yet
   seen).
  • Loading branch information
vigneshvg committed Apr 25, 2024
1 parent f56a1f1 commit 23ead7a
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 0 deletions.
4 changes: 4 additions & 0 deletions src/read.c
Original file line number Diff line number Diff line change
Expand Up @@ -3995,6 +3995,10 @@ static avifResult avifParse(avifDecoder * decoder)
}
} else if (header.size > (UINT64_MAX - parseOffset)) {
return AVIF_RESULT_BMFF_PARSE_FAILED;
} else if (header.size == 0) {
// An unknown top level box with size 0 was found. If we reach here it means we haven't completed parsing successfully
// since there are no futher boxes left.
return AVIF_RESULT_TRUNCATED_DATA;
}
parseOffset += header.size;

Expand Down
23 changes: 23 additions & 0 deletions tests/gtest/avifsize0test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: BSD-2-Clause

#include <algorithm>
#include <cstring>
#include <iostream>
#include <string>

Expand Down Expand Up @@ -89,6 +90,28 @@ TEST(AvifDecodeTest, FtypSize0) {
ASSERT_EQ(avifDecoderParse(decoder.get()), AVIF_RESULT_BMFF_PARSE_FAILED);
}

TEST(AvifDecodeTest, UnknownTopLevelBoxSize0) {
testutil::AvifRwData avif =
testutil::ReadFile(std::string(data_path) + "white_1x1.avif");
// Edit the file to insert an unknown top level box with size 0 after ftyp
// (invalid).
testutil::AvifRwData avif_edited;
ASSERT_EQ(avifRWDataRealloc(&avif_edited, avif.size + 8), AVIF_RESULT_OK);
// Copy the ftyp box.
std::memcpy(avif_edited.data, avif.data, 32);
// Set 8 bytes to 0 (box type and size all 0s).
std::memset(avif_edited.data + 32, 0, 8);
// Copy the other boxes.
std::memcpy(avif_edited.data + 40, avif.data + 32, avif.size - 32);

DecoderPtr decoder(avifDecoderCreate());
ASSERT_NE(decoder, nullptr);
ASSERT_EQ(
avifDecoderSetIOMemory(decoder.get(), avif_edited.data, avif_edited.size),
AVIF_RESULT_OK);
ASSERT_EQ(avifDecoderParse(decoder.get()), AVIF_RESULT_TRUNCATED_DATA);
}

//------------------------------------------------------------------------------

} // namespace
Expand Down

0 comments on commit 23ead7a

Please sign in to comment.