Skip to content

Commit

Permalink
Fix avifParseItemLocationBox() itemReferenceIndex (#2080)
Browse files Browse the repository at this point in the history 
Revert bug introduced with StreamReadBits().
Read item_reference_index even if construction_method!=2 to be
accurate with the ISOBMFF specification.

Add CHANGELOG.md entries. Verbose description:
* Fix 'iloc' box parsing when offset_size, length_size or
  base_offset_size was equal to 1 or 2. Such rare files were wrongly
  accepted.
* Fix 'iloc'v1/2 box parsing when base_offset_size!=0. Such rare files
  were wrongly rejected.
* Fix 'iloc'v1/2 box parsing when index_size!=0. Such rare files were
  parsed with corrupted extent offsets and sizes instead of ignoring
  index_size.
  • Loading branch information
@y-guyon
y-guyon authored Mar 30, 2024
1 parent 374e622 commit 9c6a37a
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 12 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ The changes are relative to the previous release, unless the baseline is specifi
(https://crbug.com/oss-fuzz/65657).
* ext/libjpeg.cmd now pulls libjpeg-turbo instead of libjpeg and AVIF_JPEG=LOCAL
now expects the library dependency in ext/libjpeg-turbo/build.libavif.
* Fix 'iloc' box parsing bugs that may have wrongly accepted, rejected or parsed
some files with rare values of offset_size, length_size, base_offset_size and
index_size.

## [1.0.4] - 2024-02-08

Expand Down
30 changes: 18 additions & 12 deletions src/read.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1744,23 +1744,31 @@ static avifResult avifParseItemLocationBox(avifMeta * meta, const uint8_t * raw,
{
BEGIN_STREAM(s, raw, rawLen, diag, "Box[iloc]");

// Section 8.11.3.2 of ISO/IEC 14496-12.
uint8_t version;
AVIF_CHECKERR(avifROStreamReadVersionAndFlags(&s, &version, NULL), AVIF_RESULT_BMFF_PARSE_FAILED);
if (version > 2) {
avifDiagnosticsPrintf(diag, "Box[iloc] has an unsupported version [%u]", version);
return AVIF_RESULT_BMFF_PARSE_FAILED;
}

uint8_t offsetSize, lengthSize, baseOffsetSize;
uint8_t offsetSize, lengthSize, baseOffsetSize, indexSize = 0;
uint32_t reserved;
AVIF_CHECKERR(avifROStreamReadBits8(&s, &offsetSize, /*bitCount=*/4), AVIF_RESULT_BMFF_PARSE_FAILED); // unsigned int(4) offset_size;
AVIF_CHECKERR(avifROStreamReadBits8(&s, &lengthSize, /*bitCount=*/4), AVIF_RESULT_BMFF_PARSE_FAILED); // unsigned int(4) length_size;
AVIF_CHECKERR(avifROStreamReadBits8(&s, &baseOffsetSize, /*bitCount=*/4), AVIF_RESULT_BMFF_PARSE_FAILED); // unsigned int(4) base_offset_size;
if (((version == 1) || (version == 2)) && (baseOffsetSize != 0)) {
avifDiagnosticsPrintf(diag, "Box[iloc] has an unsupported base_offset_size [%u]", baseOffsetSize);
if (version == 1 || version == 2) {
AVIF_CHECKERR(avifROStreamReadBits8(&s, &indexSize, /*bitCount=*/4), AVIF_RESULT_BMFF_PARSE_FAILED); // unsigned int(4) index_size;
} else {
AVIF_CHECKERR(avifROStreamReadBits(&s, &reserved, /*bitCount=*/4), AVIF_RESULT_BMFF_PARSE_FAILED); // unsigned int(4) reserved;
}

// Section 8.11.3.3 of ISO/IEC 14496-12.
if ((offsetSize != 0 && offsetSize != 4 && offsetSize != 8) || (lengthSize != 0 && lengthSize != 4 && lengthSize != 8) ||
(baseOffsetSize != 0 && baseOffsetSize != 4 && baseOffsetSize != 8) || (indexSize != 0 && indexSize != 4 && indexSize != 8)) {
avifDiagnosticsPrintf(diag, "Box[iloc] has an invalid size");
return AVIF_RESULT_BMFF_PARSE_FAILED;
}
uint32_t reserved;
AVIF_CHECKERR(avifROStreamReadBits(&s, &reserved, /*bitCount=*/4), AVIF_RESULT_BMFF_PARSE_FAILED); // unsigned int(4) reserved;

uint16_t tmp16;
uint32_t itemCount;
Expand Down Expand Up @@ -1796,7 +1804,7 @@ static avifResult avifParseItemLocationBox(avifMeta * meta, const uint8_t * raw,
return AVIF_RESULT_BMFF_PARSE_FAILED;
}

if ((version == 1) || (version == 2)) {
if (version == 1 || version == 2) {
AVIF_CHECKERR(avifROStreamReadBits(&s, &reserved, /*bitCount=*/12), AVIF_RESULT_BMFF_PARSE_FAILED); // unsigned int(12) reserved = 0;
if (reserved) {
avifDiagnosticsPrintf(diag, "Box[iloc] has a non null reserved field [%u]", reserved);
Expand All @@ -1805,8 +1813,8 @@ static avifResult avifParseItemLocationBox(avifMeta * meta, const uint8_t * raw,
uint8_t constructionMethod;
AVIF_CHECKERR(avifROStreamReadBits8(&s, &constructionMethod, /*bitCount=*/4),
AVIF_RESULT_BMFF_PARSE_FAILED); // unsigned int(4) construction_method;
if ((constructionMethod != 0 /* file */) && (constructionMethod != 1 /* idat */)) {
// construction method item(2) unsupported
if (constructionMethod != 0 /* file offset */ && constructionMethod != 1 /* idat offset */) {
// construction method item(2) unsupported (item offset)
avifDiagnosticsPrintf(diag, "Box[iloc] has an unsupported construction method [%u]", constructionMethod);
return AVIF_RESULT_BMFF_PARSE_FAILED;
}
Expand All @@ -1830,10 +1838,8 @@ static avifResult avifParseItemLocationBox(avifMeta * meta, const uint8_t * raw,
uint16_t extentCount; // unsigned int(16) extent_count;
AVIF_CHECKERR(avifROStreamReadU16(&s, &extentCount), AVIF_RESULT_BMFF_PARSE_FAILED); //
for (int extentIter = 0; extentIter < extentCount; ++extentIter) {
// If extent_index is ever supported, this spec must be implemented here:
// :: if (((version == 1) || (version == 2)) && (index_size > 0)) {
// :: unsigned int(index_size*8) extent_index;
// :: }
uint64_t itemReferenceIndex; // unsigned int(index_size*8) item_reference_index; (ignored unless construction_method=2)
AVIF_CHECKERR(avifROStreamReadUX8(&s, &itemReferenceIndex, indexSize), AVIF_RESULT_BMFF_PARSE_FAILED);

uint64_t extentOffset; // unsigned int(offset_size*8) extent_offset;
AVIF_CHECKERR(avifROStreamReadUX8(&s, &extentOffset, offsetSize), AVIF_RESULT_BMFF_PARSE_FAILED);
Expand Down

0 comments on commit 9c6a37a

Please sign in to comment.