Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix mem issue in avifParseColourInformationBox() #1103

Merged
merged 3 commits into from
Sep 14, 2022
Merged

Fix mem issue in avifParseColourInformationBox() #1103

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
73a3693
Fix mem issue in avifParseColourInformationBox()
y-guyon Sep 12, 2022
a51339b
Use uint64_t offsets
y-guyon Sep 12, 2022
d2bb7ce
Swap rawOffset,raw args in read.c functions
y-guyon Sep 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 53 additions & 24 deletions src/read.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ typedef struct avifContentType
typedef struct avifColourInformationBox
{
avifBool hasICC;
const uint8_t * icc;
uint64_t iccOffset;
size_t iccSize;

avifBool hasNCLX;
Expand Down Expand Up @@ -1728,7 +1728,7 @@ static avifBool avifParseAuxiliaryTypeProperty(avifProperty * prop, const uint8_
return AVIF_TRUE;
}

static avifBool avifParseColourInformationBox(avifProperty * prop, const uint8_t * raw, size_t rawLen, avifDiagnostics * diag)
static avifBool avifParseColourInformationBox(avifProperty * prop, uint64_t rawOffset, const uint8_t * raw, size_t rawLen, avifDiagnostics * diag)
{
BEGIN_STREAM(s, raw, rawLen, diag, "Box[colr]");

Expand All @@ -1740,7 +1740,10 @@ static avifBool avifParseColourInformationBox(avifProperty * prop, const uint8_t
AVIF_CHECK(avifROStreamRead(&s, colorType, 4));
if (!memcmp(colorType, "rICC", 4) || !memcmp(colorType, "prof", 4)) {
colr->hasICC = AVIF_TRUE;
colr->icc = avifROStreamCurrent(&s);
// Remember the offset of the ICC payload relative to the beginning of the stream. A direct pointer cannot be stored
// because decoder->io->persistent could have been AVIF_FALSE when obtaining raw through decoder->io->read().
// The bytes could be copied now instead of remembering the offset, but it is as invasive as passing rawOffset everywhere.
colr->iccOffset = rawOffset + avifROStreamOffset(&s);
y-guyon marked this conversation as resolved.
Show resolved Hide resolved
colr->iccSize = avifROStreamRemainingBytes(&s);
} else if (!memcmp(colorType, "nclx", 4)) {
AVIF_CHECK(avifROStreamReadU16(&s, &colr->colorPrimaries)); // unsigned int(16) colour_primaries;
Expand Down Expand Up @@ -1914,7 +1917,11 @@ static avifBool avifParseAV1LayeredImageIndexingProperty(avifProperty * prop, co
return AVIF_TRUE;
}

static avifBool avifParseItemPropertyContainerBox(avifPropertyArray * properties, const uint8_t * raw, size_t rawLen, avifDiagnostics * diag)
static avifBool avifParseItemPropertyContainerBox(avifPropertyArray * properties,
uint64_t rawOffset,
const uint8_t * raw,
size_t rawLen,
avifDiagnostics * diag)
{
BEGIN_STREAM(s, raw, rawLen, diag, "Box[ipco]");

Expand All @@ -1930,7 +1937,7 @@ static avifBool avifParseItemPropertyContainerBox(avifPropertyArray * properties
} else if (!memcmp(header.type, "auxC", 4)) {
AVIF_CHECK(avifParseAuxiliaryTypeProperty(prop, avifROStreamCurrent(&s), header.size, diag));
} else if (!memcmp(header.type, "colr", 4)) {
AVIF_CHECK(avifParseColourInformationBox(prop, avifROStreamCurrent(&s), header.size, diag));
AVIF_CHECK(avifParseColourInformationBox(prop, rawOffset + avifROStreamOffset(&s), avifROStreamCurrent(&s), header.size, diag));
} else if (!memcmp(header.type, "av1C", 4)) {
AVIF_CHECK(avifParseAV1CodecConfigurationBoxProperty(prop, avifROStreamCurrent(&s), header.size, diag));
} else if (!memcmp(header.type, "pasp", 4)) {
Expand Down Expand Up @@ -2145,7 +2152,7 @@ static avifBool avifParseItemDataBox(avifMeta * meta, const uint8_t * raw, size_
return AVIF_TRUE;
}

static avifBool avifParseItemPropertiesBox(avifMeta * meta, const uint8_t * raw, size_t rawLen, avifDiagnostics * diag)
static avifBool avifParseItemPropertiesBox(avifMeta * meta, uint64_t rawOffset, const uint8_t * raw, size_t rawLen, avifDiagnostics * diag)
{
BEGIN_STREAM(s, raw, rawLen, diag, "Box[iprp]");

Expand All @@ -2157,7 +2164,11 @@ static avifBool avifParseItemPropertiesBox(avifMeta * meta, const uint8_t * raw,
}

// Read all item properties inside of ItemPropertyContainerBox
AVIF_CHECK(avifParseItemPropertyContainerBox(&meta->properties, avifROStreamCurrent(&s), ipcoHeader.size, diag));
AVIF_CHECK(avifParseItemPropertyContainerBox(&meta->properties,
rawOffset + avifROStreamOffset(&s),
avifROStreamCurrent(&s),
ipcoHeader.size,
diag));
AVIF_CHECK(avifROStreamSkip(&s, ipcoHeader.size));

uint32_t versionAndFlagsSeen[MAX_IPMA_VERSION_AND_FLAGS_SEEN];
Expand Down Expand Up @@ -2362,7 +2373,7 @@ static avifBool avifParseItemReferenceBox(avifMeta * meta, const uint8_t * raw,
return AVIF_TRUE;
}

static avifBool avifParseMetaBox(avifMeta * meta, const uint8_t * raw, size_t rawLen, avifDiagnostics * diag)
static avifBool avifParseMetaBox(avifMeta * meta, uint64_t rawOffset, const uint8_t * raw, size_t rawLen, avifDiagnostics * diag)
{
BEGIN_STREAM(s, raw, rawLen, diag, "Box[meta]");

Expand Down Expand Up @@ -2397,7 +2408,7 @@ static avifBool avifParseMetaBox(avifMeta * meta, const uint8_t * raw, size_t ra
AVIF_CHECK(avifParseItemDataBox(meta, avifROStreamCurrent(&s), header.size, diag));
} else if (!memcmp(header.type, "iprp", 4)) {
AVIF_CHECK(uniqueBoxSeen(&uniqueBoxFlags, 4, "meta", "iprp", diag));
AVIF_CHECK(avifParseItemPropertiesBox(meta, avifROStreamCurrent(&s), header.size, diag));
AVIF_CHECK(avifParseItemPropertiesBox(meta, rawOffset + avifROStreamOffset(&s), avifROStreamCurrent(&s), header.size, diag));
} else if (!memcmp(header.type, "iinf", 4)) {
AVIF_CHECK(uniqueBoxSeen(&uniqueBoxFlags, 5, "meta", "iinf", diag));
AVIF_CHECK(avifParseItemInfoBox(meta, avifROStreamCurrent(&s), header.size, diag));
Expand Down Expand Up @@ -2621,7 +2632,11 @@ static avifBool avifParseTimeToSampleBox(avifSampleTable * sampleTable, const ui
return AVIF_TRUE;
}

static avifBool avifParseSampleDescriptionBox(avifSampleTable * sampleTable, const uint8_t * raw, size_t rawLen, avifDiagnostics * diag)
static avifBool avifParseSampleDescriptionBox(avifSampleTable * sampleTable,
uint64_t rawOffset,
const uint8_t * raw,
size_t rawLen,
avifDiagnostics * diag)
{
BEGIN_STREAM(s, raw, rawLen, diag, "Box[stsd]");

Expand All @@ -2643,6 +2658,7 @@ static avifBool avifParseSampleDescriptionBox(avifSampleTable * sampleTable, con
size_t remainingBytes = avifROStreamRemainingBytes(&s);
if (!memcmp(description->format, "av01", 4) && (remainingBytes > VISUALSAMPLEENTRY_SIZE)) {
AVIF_CHECK(avifParseItemPropertyContainerBox(&description->properties,
rawOffset + avifROStreamOffset(&s) + VISUALSAMPLEENTRY_SIZE,
avifROStreamCurrent(&s) + VISUALSAMPLEENTRY_SIZE,
remainingBytes - VISUALSAMPLEENTRY_SIZE,
diag));
Expand All @@ -2653,7 +2669,7 @@ static avifBool avifParseSampleDescriptionBox(avifSampleTable * sampleTable, con
return AVIF_TRUE;
}

static avifBool avifParseSampleTableBox(avifTrack * track, const uint8_t * raw, size_t rawLen, avifDiagnostics * diag)
static avifBool avifParseSampleTableBox(avifTrack * track, uint64_t rawOffset, const uint8_t * raw, size_t rawLen, avifDiagnostics * diag)
{
if (track->sampleTable) {
// A TrackBox may only have one SampleTable
Expand Down Expand Up @@ -2681,15 +2697,19 @@ static avifBool avifParseSampleTableBox(avifTrack * track, const uint8_t * raw,
} else if (!memcmp(header.type, "stts", 4)) {
AVIF_CHECK(avifParseTimeToSampleBox(track->sampleTable, avifROStreamCurrent(&s), header.size, diag));
} else if (!memcmp(header.type, "stsd", 4)) {
AVIF_CHECK(avifParseSampleDescriptionBox(track->sampleTable, avifROStreamCurrent(&s), header.size, diag));
AVIF_CHECK(avifParseSampleDescriptionBox(track->sampleTable,
rawOffset + avifROStreamOffset(&s),
avifROStreamCurrent(&s),
header.size,
diag));
}

AVIF_CHECK(avifROStreamSkip(&s, header.size));
}
return AVIF_TRUE;
}

static avifBool avifParseMediaInformationBox(avifTrack * track, const uint8_t * raw, size_t rawLen, avifDiagnostics * diag)
static avifBool avifParseMediaInformationBox(avifTrack * track, uint64_t rawOffset, const uint8_t * raw, size_t rawLen, avifDiagnostics * diag)
{
BEGIN_STREAM(s, raw, rawLen, diag, "Box[minf]");

Expand All @@ -2698,15 +2718,15 @@ static avifBool avifParseMediaInformationBox(avifTrack * track, const uint8_t *
AVIF_CHECK(avifROStreamReadBoxHeader(&s, &header));

if (!memcmp(header.type, "stbl", 4)) {
AVIF_CHECK(avifParseSampleTableBox(track, avifROStreamCurrent(&s), header.size, diag));
AVIF_CHECK(avifParseSampleTableBox(track, rawOffset + avifROStreamOffset(&s), avifROStreamCurrent(&s), header.size, diag));
}

AVIF_CHECK(avifROStreamSkip(&s, header.size));
}
return AVIF_TRUE;
}

static avifBool avifParseMediaBox(avifTrack * track, const uint8_t * raw, size_t rawLen, avifDiagnostics * diag)
static avifBool avifParseMediaBox(avifTrack * track, uint64_t rawOffset, const uint8_t * raw, size_t rawLen, avifDiagnostics * diag)
{
BEGIN_STREAM(s, raw, rawLen, diag, "Box[mdia]");

Expand All @@ -2717,7 +2737,7 @@ static avifBool avifParseMediaBox(avifTrack * track, const uint8_t * raw, size_t
if (!memcmp(header.type, "mdhd", 4)) {
AVIF_CHECK(avifParseMediaHeaderBox(track, avifROStreamCurrent(&s), header.size, diag));
} else if (!memcmp(header.type, "minf", 4)) {
AVIF_CHECK(avifParseMediaInformationBox(track, avifROStreamCurrent(&s), header.size, diag));
AVIF_CHECK(avifParseMediaInformationBox(track, rawOffset + avifROStreamOffset(&s), avifROStreamCurrent(&s), header.size, diag));
}

AVIF_CHECK(avifROStreamSkip(&s, header.size));
Expand Down Expand Up @@ -2750,7 +2770,7 @@ static avifBool avifTrackReferenceBox(avifTrack * track, const uint8_t * raw, si
return AVIF_TRUE;
}

static avifBool avifParseTrackBox(avifDecoderData * data, const uint8_t * raw, size_t rawLen, uint32_t imageSizeLimit, uint32_t imageDimensionLimit)
static avifBool avifParseTrackBox(avifDecoderData * data, uint64_t rawOffset, const uint8_t * raw, size_t rawLen, uint32_t imageSizeLimit, uint32_t imageDimensionLimit)
{
BEGIN_STREAM(s, raw, rawLen, data->diag, "Box[trak]");

Expand All @@ -2763,9 +2783,9 @@ static avifBool avifParseTrackBox(avifDecoderData * data, const uint8_t * raw, s
if (!memcmp(header.type, "tkhd", 4)) {
AVIF_CHECK(avifParseTrackHeaderBox(track, avifROStreamCurrent(&s), header.size, imageSizeLimit, imageDimensionLimit, data->diag));
} else if (!memcmp(header.type, "meta", 4)) {
AVIF_CHECK(avifParseMetaBox(track->meta, avifROStreamCurrent(&s), header.size, data->diag));
AVIF_CHECK(avifParseMetaBox(track->meta, rawOffset + avifROStreamOffset(&s), avifROStreamCurrent(&s), header.size, data->diag));
} else if (!memcmp(header.type, "mdia", 4)) {
AVIF_CHECK(avifParseMediaBox(track, avifROStreamCurrent(&s), header.size, data->diag));
AVIF_CHECK(avifParseMediaBox(track, rawOffset + avifROStreamOffset(&s), avifROStreamCurrent(&s), header.size, data->diag));
} else if (!memcmp(header.type, "tref", 4)) {
AVIF_CHECK(avifTrackReferenceBox(track, avifROStreamCurrent(&s), header.size, data->diag));
}
Expand All @@ -2775,7 +2795,7 @@ static avifBool avifParseTrackBox(avifDecoderData * data, const uint8_t * raw, s
return AVIF_TRUE;
}

static avifBool avifParseMovieBox(avifDecoderData * data, const uint8_t * raw, size_t rawLen, uint32_t imageSizeLimit, uint32_t imageDimensionLimit)
static avifBool avifParseMovieBox(avifDecoderData * data, uint64_t rawOffset, const uint8_t * raw, size_t rawLen, uint32_t imageSizeLimit, uint32_t imageDimensionLimit)
{
BEGIN_STREAM(s, raw, rawLen, data->diag, "Box[moov]");

Expand All @@ -2784,7 +2804,8 @@ static avifBool avifParseMovieBox(avifDecoderData * data, const uint8_t * raw, s
AVIF_CHECK(avifROStreamReadBoxHeader(&s, &header));

if (!memcmp(header.type, "trak", 4)) {
AVIF_CHECK(avifParseTrackBox(data, avifROStreamCurrent(&s), header.size, imageSizeLimit, imageDimensionLimit));
AVIF_CHECK(
avifParseTrackBox(data, rawOffset + avifROStreamOffset(&s), avifROStreamCurrent(&s), header.size, imageSizeLimit, imageDimensionLimit));
}

AVIF_CHECK(avifROStreamSkip(&s, header.size));
Expand Down Expand Up @@ -2852,10 +2873,12 @@ static avifResult avifParse(avifDecoder * decoder)
assert((decoder->io->sizeHint == 0) || (parseOffset <= decoder->io->sizeHint));

// Try to get the remainder of the box, if necessary
uint64_t boxOffset = 0;
avifROData boxContents = AVIF_DATA_EMPTY;

// TODO: reorg this code to only do these memcmps once each
if (!memcmp(header.type, "ftyp", 4) || !memcmp(header.type, "meta", 4) || !memcmp(header.type, "moov", 4)) {
boxOffset = parseOffset;
readResult = decoder->io->read(decoder->io, 0, parseOffset, header.size, &boxContents);
if (readResult != AVIF_RESULT_OK) {
return readResult;
Expand All @@ -2882,11 +2905,12 @@ static avifResult avifParse(avifDecoder * decoder)
needsMoov = avifFileTypeHasBrand(&ftyp, "avis");
} else if (!memcmp(header.type, "meta", 4)) {
AVIF_CHECKERR(!metaSeen, AVIF_RESULT_BMFF_PARSE_FAILED);
AVIF_CHECKERR(avifParseMetaBox(data->meta, boxContents.data, boxContents.size, data->diag), AVIF_RESULT_BMFF_PARSE_FAILED);
AVIF_CHECKERR(avifParseMetaBox(data->meta, boxOffset, boxContents.data, boxContents.size, data->diag),
AVIF_RESULT_BMFF_PARSE_FAILED);
metaSeen = AVIF_TRUE;
} else if (!memcmp(header.type, "moov", 4)) {
AVIF_CHECKERR(!moovSeen, AVIF_RESULT_BMFF_PARSE_FAILED);
AVIF_CHECKERR(avifParseMovieBox(data, boxContents.data, boxContents.size, decoder->imageSizeLimit, decoder->imageDimensionLimit),
AVIF_CHECKERR(avifParseMovieBox(data, boxOffset, boxContents.data, boxContents.size, decoder->imageSizeLimit, decoder->imageDimensionLimit),
AVIF_RESULT_BMFF_PARSE_FAILED);
moovSeen = AVIF_TRUE;
}
Expand Down Expand Up @@ -3637,8 +3661,13 @@ avifResult avifDecoderReset(avifDecoder * decoder)
if (colrICCSeen) {
return AVIF_RESULT_BMFF_PARSE_FAILED;
}
avifROData icc;
const avifResult readResult = decoder->io->read(decoder->io, 0, prop->u.colr.iccOffset, prop->u.colr.iccSize, &icc);
if (readResult != AVIF_RESULT_OK) {
return readResult;
}
colrICCSeen = AVIF_TRUE;
avifImageSetProfileICC(decoder->image, prop->u.colr.icc, prop->u.colr.iccSize);
avifImageSetProfileICC(decoder->image, icc.data, icc.size);
}
if (prop->u.colr.hasNCLX) {
if (colrNCLXSeen) {
Expand Down