From 397f74c8e289386eb7d309b2f8041d8a190db29a Mon Sep 17 00:00:00 2001 From: Yannis Guyon Date: Mon, 31 Jul 2023 11:49:15 +0200 Subject: [PATCH] avifRWDataRealloc,avifRWDataSet return avifResult Also avifImageSetProfileICC(), avifImageSetMetadataExif(), avifImageSetMetadataXMP() and avifCodecEncodeOutputAddSample(). To catch out-of-memory issues. Bug: #820 --- CHANGELOG.md | 8 ++++++-- apps/avifdec.c | 5 ++++- apps/avifenc.c | 19 +++++++++--------- apps/shared/avifjpeg.c | 35 ++++++++++++++++++++++++++------- apps/shared/avifpng.c | 25 ++++++++++++++++++----- apps/shared/iccmaker.c | 10 ++++++---- apps/shared/y4m.c | 5 ++++- include/avif/avif.h | 11 ++++++----- include/avif/internal.h | 2 +- src/avif.c | 14 ++++++------- src/codec_aom.c | 12 +++++++++-- src/codec_avm.c | 12 +++++++++-- src/codec_rav1e.c | 10 ++++++++-- src/codec_svt.c | 12 +++++++---- src/exif.c | 6 ++++-- src/io.c | 12 +++++++++-- src/mem.c | 1 + src/rawdata.c | 25 ++++++++++++----------- src/read.c | 14 +++++++------ src/stream.c | 6 ++++-- src/write.c | 18 ++++++++++++----- tests/aviftest.c | 16 +++++++++++---- tests/gtest/avifincrtest.cc | 6 ++++-- tests/gtest/avifmetadatatest.cc | 30 +++++++++++++++------------- 24 files changed, 213 insertions(+), 101 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec97e07c51..39a0c33b67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,9 @@ List of incompatible ABI changes in this release: those pointers. * Check the return value of avifEncoderSetCodecSpecificOption(). * The maxThreads member was added to the avifRGBImage struct. -* Check the return value of avifRGBImageAllocatePixels(). +* Check the return value of avifRGBImageAllocatePixels(), avifRWDataRealloc(), + avifRWDataSet(), avifImageSetProfileICC(), avifImageSetMetadataExif() and + avifImageSetMetadataXMP(). * The meaning of the keyframeInterval member of avifEncoder struct has changed slightly. When set to a value of "n", * Before: It forces a keyframe on every nth frame. @@ -72,7 +74,9 @@ List of incompatible ABI changes in this release: * avifEncoderSetCodecSpecificOption() now returns avifResult instead of void to report memory allocation failures. * At decoding, avifIOStats now returns the same values as at encoding. -* avifRGBImageAllocatePixels() now returns avifResult instead of void to report +* avifRGBImageAllocatePixels(), avifRWDataRealloc(), avifRWDataSet(), + avifImageSetProfileICC(), avifImageSetMetadataExif() and + avifImageSetMetadataXMP() now return avifResult instead of void to report memory allocation failures. * avifReadImage(), avifJPEGRead() and avifPNGRead() now remove the trailing zero byte from read XMP chunks, if any. See avifImageFixXMP(). diff --git a/apps/avifdec.c b/apps/avifdec.c index a0a025137a..b360e06e49 100644 --- a/apps/avifdec.c +++ b/apps/avifdec.c @@ -8,6 +8,7 @@ #include "avifutil.h" #include "y4m.h" +#include #include #include #include @@ -331,7 +332,9 @@ int main(int argc, char * argv[]) if (ignoreICC && (decoder->image->icc.size > 0)) { printf("[--ignore-icc] Discarding ICC profile.\n"); - avifImageSetProfileICC(decoder->image, NULL, 0); + // This cannot fail. + result = avifImageSetProfileICC(decoder->image, NULL, 0); + assert(result == AVIF_RESULT_OK); } avifAppFileFormat outputFormat = avifGuessFileFormat(outputFilename); diff --git a/apps/avifenc.c b/apps/avifenc.c index a5c7c9eff2..c9c1e0df92 100644 --- a/apps/avifenc.c +++ b/apps/avifenc.c @@ -484,7 +484,10 @@ static avifBool readEntireFile(const char * filename, avifRWData * raw) size_t fileSize = (size_t)pos; fseek(f, 0, SEEK_SET); - avifRWDataRealloc(raw, fileSize); + if (avifRWDataRealloc(raw, fileSize) != AVIF_RESULT_OK) { + fclose(f); + return AVIF_FALSE; + } size_t bytesRead = fread(raw->data, 1, fileSize, f); fclose(f); @@ -1687,14 +1690,12 @@ int main(int argc, char * argv[]) } } - if (iccOverride.size) { - avifImageSetProfileICC(image, iccOverride.data, iccOverride.size); - } - if (exifOverride.size) { - avifImageSetMetadataExif(image, exifOverride.data, exifOverride.size); - } - if (xmpOverride.size) { - avifImageSetMetadataXMP(image, xmpOverride.data, xmpOverride.size); + if ((iccOverride.size && (avifImageSetProfileICC(image, iccOverride.data, iccOverride.size) != AVIF_RESULT_OK)) || + (exifOverride.size && (avifImageSetMetadataExif(image, exifOverride.data, exifOverride.size) != AVIF_RESULT_OK)) || + (xmpOverride.size && (avifImageSetMetadataXMP(image, xmpOverride.data, xmpOverride.size) != AVIF_RESULT_OK))) { + fprintf(stderr, "Error when setting overridden metadata: out of memory.\n"); + returnCode = 1; + goto cleanup; } if (!image->icc.size && !cicpExplicitlySet && (image->colorPrimaries == AVIF_COLOR_PRIMARIES_UNSPECIFIED) && diff --git a/apps/shared/avifjpeg.c b/apps/shared/avifjpeg.c index 5f3998c735..de079a8197 100644 --- a/apps/shared/avifjpeg.c +++ b/apps/shared/avifjpeg.c @@ -330,7 +330,10 @@ avifBool avifJPEGRead(const char * inputFilename, unsigned int iccDataLen; if (read_icc_profile(&cinfo, &iccDataTmp, &iccDataLen)) { iccData = iccDataTmp; - avifImageSetProfileICC(avif, iccDataTmp, (size_t)iccDataLen); + if (avifImageSetProfileICC(avif, iccDataTmp, (size_t)iccDataLen) != AVIF_RESULT_OK) { + fprintf(stderr, "Setting ICC profile failed: %s (out of memory)\n", inputFilename); + goto cleanup; + } } } @@ -418,7 +421,10 @@ avifBool avifJPEGRead(const char * inputFilename, // Exif orientation, if any, is imported to avif->irot/imir and kept in avif->exif. // libheif has the same behavior, see // https://github.com/strukturag/libheif/blob/ea78603d8e47096606813d221725621306789ff2/examples/heif_enc.cc#L403 - avifImageSetMetadataExif(avif, marker->data + tagExif.size, marker->data_length - tagExif.size); + if (avifImageSetMetadataExif(avif, marker->data + tagExif.size, marker->data_length - tagExif.size) != AVIF_RESULT_OK) { + fprintf(stderr, "Setting Exif metadata failed: %s (out of memory)\n", inputFilename); + goto cleanup; + } found = AVIF_TRUE; } } @@ -493,11 +499,17 @@ avifBool avifJPEGRead(const char * inputFilename, } else { memcpy(extendedXMPGUID, guid, AVIF_JPEG_EXTENDED_XMP_GUID_LENGTH); - avifRWDataRealloc(&totalXMP, (size_t)standardXMPSize + totalExtendedXMPSize); + if (avifRWDataRealloc(&totalXMP, (size_t)standardXMPSize + totalExtendedXMPSize) != AVIF_RESULT_OK) { + fprintf(stderr, "XMP extraction failed: out of memory\n"); + goto cleanup; + } memcpy(totalXMP.data, standardXMPData, standardXMPSize); // Keep track of the bytes that were set. - avifRWDataRealloc(&extendedXMPReadBytes, totalExtendedXMPSize); + if (avifRWDataRealloc(&extendedXMPReadBytes, totalExtendedXMPSize) != AVIF_RESULT_OK) { + fprintf(stderr, "XMP extraction failed: out of memory\n"); + goto cleanup; + } memset(extendedXMPReadBytes.data, 0, extendedXMPReadBytes.size); foundExtendedXMP = AVIF_TRUE; @@ -549,7 +561,10 @@ avifBool avifJPEGRead(const char * inputFilename, totalXMP.data = NULL; totalXMP.size = 0; } else if (standardXMPData) { - avifImageSetMetadataXMP(avif, standardXMPData, standardXMPSize); + if (avifImageSetMetadataXMP(avif, standardXMPData, standardXMPSize) != AVIF_RESULT_OK) { + fprintf(stderr, "XMP extraction failed: out of memory\n"); + goto cleanup; + } } avifImageFixXMP(avif); // Remove one trailing null character if any. } @@ -619,7 +634,10 @@ avifBool avifJPEGWrite(const char * outputFilename, const avifImage * avif, int } avifRWData exif = { NULL, 0 }; - avifRWDataRealloc(&exif, AVIF_JPEG_EXIF_HEADER_LENGTH + avif->exif.size - exifTiffHeaderOffset); + if (avifRWDataRealloc(&exif, AVIF_JPEG_EXIF_HEADER_LENGTH + avif->exif.size - exifTiffHeaderOffset) != AVIF_RESULT_OK) { + fprintf(stderr, "Error writing JPEG metadata: out of memory\n"); + goto cleanup; + } memcpy(exif.data, AVIF_JPEG_EXIF_HEADER, AVIF_JPEG_EXIF_HEADER_LENGTH); memcpy(exif.data + AVIF_JPEG_EXIF_HEADER_LENGTH, avif->exif.data + exifTiffHeaderOffset, avif->exif.size - exifTiffHeaderOffset); // Make sure the Exif orientation matches the irot/imir values. @@ -665,7 +683,10 @@ avifBool avifJPEGWrite(const char * outputFilename, const avifImage * avif, int fprintf(stderr, "Warning writing JPEG metadata: XMP payload is too big and was dropped\n"); } else { avifRWData xmp = { NULL, 0 }; - avifRWDataRealloc(&xmp, AVIF_JPEG_STANDARD_XMP_TAG_LENGTH + avif->xmp.size); + if (avifRWDataRealloc(&xmp, AVIF_JPEG_STANDARD_XMP_TAG_LENGTH + avif->xmp.size) != AVIF_RESULT_OK) { + fprintf(stderr, "Error writing JPEG metadata: out of memory\n"); + goto cleanup; + } memcpy(xmp.data, AVIF_JPEG_STANDARD_XMP_TAG, AVIF_JPEG_STANDARD_XMP_TAG_LENGTH); memcpy(xmp.data + AVIF_JPEG_STANDARD_XMP_TAG_LENGTH, avif->xmp.data, avif->xmp.size); jpeg_write_marker(&cinfo, JPEG_APP0 + 1, xmp.data, (unsigned int)xmp.size); diff --git a/apps/shared/avifpng.c b/apps/shared/avifpng.c index c7a051ab6d..fc256062e9 100644 --- a/apps/shared/avifpng.c +++ b/apps/shared/avifpng.c @@ -28,7 +28,10 @@ // AVIF_FALSE is returned if fewer than numExpectedBytes hexadecimal pairs are converted. static avifBool avifHexStringToBytes(const char * hexString, size_t hexStringLength, size_t numExpectedBytes, avifRWData * bytes) { - avifRWDataRealloc(bytes, numExpectedBytes); + if (avifRWDataRealloc(bytes, numExpectedBytes) != AVIF_RESULT_OK) { + fprintf(stderr, "Metadata extraction failed: out of memory\n"); + return AVIF_FALSE; + } size_t numBytes = 0; for (size_t i = 0; (i + 1 < hexStringLength) && (numBytes < numExpectedBytes);) { if (hexString[i] == '\n') { @@ -124,7 +127,10 @@ static avifBool avifExtractExifAndXMP(png_structp png, png_infop info, avifBool return AVIF_FALSE; } // Avoid avifImageSetMetadataExif() that sets irot/imir. - avifRWDataSet(&avif->exif, exif, exifSize); + if (avifRWDataSet(&avif->exif, exif, exifSize) != AVIF_RESULT_OK) { + fprintf(stderr, "Exif extraction failed: out of memory\n"); + return AVIF_FALSE; + } // According to the Extensions to the PNG 1.2 Specification, Version 1.5.0, section 3.7: // "It is recommended that unless a decoder has independent knowledge of the validity of the Exif data, // the data should be considered to be of historical value only." @@ -190,7 +196,10 @@ static avifBool avifExtractExifAndXMP(png_structp png, png_infop info, avifBool fprintf(stderr, "XMP extraction failed: empty XML:com.adobe.xmp payload\n"); return AVIF_FALSE; } - avifImageSetMetadataXMP(avif, (const uint8_t *)text->text, textLength); + if (avifImageSetMetadataXMP(avif, (const uint8_t *)text->text, textLength) != AVIF_RESULT_OK) { + fprintf(stderr, "XMP extraction failed: out of memory\n"); + return AVIF_FALSE; + } *ignoreXMP = AVIF_TRUE; // Ignore any other XMP chunk. } } @@ -361,7 +370,10 @@ avifBool avifPNGRead(const char * inputFilename, // When the sRGB / iCCP chunk is present, applications that recognize it and are capable of color management // must ignore the gAMA and cHRM chunks and use the sRGB / iCCP chunk instead. if (png_get_iCCP(png, info, &iccpProfileName, &iccpCompression, &iccpData, &iccpDataLen) == PNG_INFO_iCCP) { - avifImageSetProfileICC(avif, iccpData, iccpDataLen); + if (avifImageSetProfileICC(avif, iccpData, iccpDataLen) != AVIF_RESULT_OK) { + fprintf(stderr, "Setting ICC profile failed: out of memory.\n"); + goto cleanup; + } } else if (allowChangingCicp) { if (png_get_sRGB(png, info, &srgbIntent) == PNG_INFO_sRGB) { // srgbIntent ignored @@ -653,7 +665,10 @@ avifBool avifPNGWrite(const char * outputFilename, const avifImage * avif, uint3 fprintf(stderr, "Error writing PNG: XMP metadata is too big\n"); goto cleanup; } - avifRWDataRealloc(&xmp, avif->xmp.size + 1); + if (avifRWDataRealloc(&xmp, avif->xmp.size + 1) != AVIF_RESULT_OK) { + fprintf(stderr, "Error writing PNG: out of memory\n"); + goto cleanup; + } memcpy(xmp.data, avif->xmp.data, avif->xmp.size); xmp.data[avif->xmp.size] = '\0'; png_text * text = &texts[numTextMetadataChunks++]; diff --git a/apps/shared/iccmaker.c b/apps/shared/iccmaker.c index 7499253ac2..bcfec28448 100644 --- a/apps/shared/iccmaker.c +++ b/apps/shared/iccmaker.c @@ -444,8 +444,9 @@ avifBool avifGenerateRGBICC(avifRWData * icc, float gamma, const float primaries } computeMD5(buffer, sizeof(iccColorTemplate)); - avifRWDataRealloc(icc, iccColorLength); - memcpy(icc->data, buffer, iccColorLength); + if (avifRWDataSet(icc, buffer, iccColorLength) != AVIF_RESULT_OK) { + return AVIF_FALSE; + } return AVIF_TRUE; } @@ -469,8 +470,9 @@ avifBool avifGenerateGrayICC(avifRWData * icc, float gamma, const float white[2] } computeMD5(buffer, sizeof(iccGrayTemplate)); - avifRWDataRealloc(icc, iccGrayLength); - memcpy(icc->data, buffer, iccGrayLength); + if (avifRWDataSet(icc, buffer, iccGrayLength) != AVIF_RESULT_OK) { + return AVIF_FALSE; + } return AVIF_TRUE; } diff --git a/apps/shared/y4m.c b/apps/shared/y4m.c index b573a30aa6..51e9353c1a 100644 --- a/apps/shared/y4m.c +++ b/apps/shared/y4m.c @@ -261,7 +261,10 @@ avifBool y4mRead(const char * inputFilename, avifImage * avif, avifAppSourceTimi frame.displayFilename = inputFilename; avifRWData raw = AVIF_DATA_EMPTY; - avifRWDataRealloc(&raw, Y4M_MAX_LINE_SIZE); + if (avifRWDataRealloc(&raw, Y4M_MAX_LINE_SIZE) != AVIF_RESULT_OK) { + fprintf(stderr, "Out of memory\n"); + goto cleanup; + } if (iter && *iter) { // Continue reading FRAMEs from this y4m stream diff --git a/include/avif/avif.h b/include/avif/avif.h index e2f7757679..d2caef9353 100644 --- a/include/avif/avif.h +++ b/include/avif/avif.h @@ -199,8 +199,9 @@ typedef struct avifRWData // clang-format on // The avifRWData input must be zero-initialized before being manipulated with these functions. -AVIF_API void avifRWDataRealloc(avifRWData * raw, size_t newSize); -AVIF_API void avifRWDataSet(avifRWData * raw, const uint8_t * data, size_t len); +// If AVIF_RESULT_OUT_OF_MEMORY is returned, raw is left unchanged. +AVIF_API avifResult avifRWDataRealloc(avifRWData * raw, size_t newSize); +AVIF_API avifResult avifRWDataSet(avifRWData * raw, const uint8_t * data, size_t len); AVIF_API void avifRWDataFree(avifRWData * raw); // --------------------------------------------------------------------------- @@ -565,14 +566,14 @@ AVIF_API avifResult avifImageCopy(avifImage * dstImage, const avifImage * srcIma AVIF_API avifResult avifImageSetViewRect(avifImage * dstImage, const avifImage * srcImage, const avifCropRect * rect); // shallow copy, no metadata AVIF_API void avifImageDestroy(avifImage * image); -AVIF_API void avifImageSetProfileICC(avifImage * image, const uint8_t * icc, size_t iccSize); +AVIF_API avifResult avifImageSetProfileICC(avifImage * image, const uint8_t * icc, size_t iccSize); // Sets Exif metadata. Attempts to parse the Exif metadata for Exif orientation. Sets // image->transformFlags, image->irot and image->imir if the Exif metadata is parsed successfully, // otherwise leaves image->transformFlags, image->irot and image->imir unchanged. // Warning: If the Exif payload is set and invalid, avifEncoderWrite() may return AVIF_RESULT_INVALID_EXIF_PAYLOAD. -AVIF_API void avifImageSetMetadataExif(avifImage * image, const uint8_t * exif, size_t exifSize); +AVIF_API avifResult avifImageSetMetadataExif(avifImage * image, const uint8_t * exif, size_t exifSize); // Sets XMP metadata. -AVIF_API void avifImageSetMetadataXMP(avifImage * image, const uint8_t * xmp, size_t xmpSize); +AVIF_API avifResult avifImageSetMetadataXMP(avifImage * image, const uint8_t * xmp, size_t xmpSize); AVIF_API avifResult avifImageAllocatePlanes(avifImage * image, avifPlanesFlags planes); // Ignores any pre-existing planes AVIF_API void avifImageFreePlanes(avifImage * image, avifPlanesFlags planes); // Ignores already-freed planes diff --git a/include/avif/internal.h b/include/avif/internal.h index 92229489ee..8b58687f04 100644 --- a/include/avif/internal.h +++ b/include/avif/internal.h @@ -285,7 +285,7 @@ typedef struct avifCodecEncodeOutput } avifCodecEncodeOutput; avifCodecEncodeOutput * avifCodecEncodeOutputCreate(void); -void avifCodecEncodeOutputAddSample(avifCodecEncodeOutput * encodeOutput, const uint8_t * data, size_t len, avifBool sync); +avifResult avifCodecEncodeOutputAddSample(avifCodecEncodeOutput * encodeOutput, const uint8_t * data, size_t len, avifBool sync); void avifCodecEncodeOutputDestroy(avifCodecEncodeOutput * encodeOutput); // --------------------------------------------------------------------------- diff --git a/src/avif.c b/src/avif.c index 6fd3665dcb..3a53342ed4 100644 --- a/src/avif.c +++ b/src/avif.c @@ -227,10 +227,10 @@ avifResult avifImageCopy(avifImage * dstImage, const avifImage * srcImage, avifP avifImageFreePlanes(dstImage, AVIF_PLANES_ALL); avifImageCopyNoAlloc(dstImage, srcImage); - avifImageSetProfileICC(dstImage, srcImage->icc.data, srcImage->icc.size); + AVIF_CHECKRES(avifImageSetProfileICC(dstImage, srcImage->icc.data, srcImage->icc.size)); - avifRWDataSet(&dstImage->exif, srcImage->exif.data, srcImage->exif.size); - avifImageSetMetadataXMP(dstImage, srcImage->xmp.data, srcImage->xmp.size); + AVIF_CHECKRES(avifRWDataSet(&dstImage->exif, srcImage->exif.data, srcImage->exif.size)); + AVIF_CHECKRES(avifImageSetMetadataXMP(dstImage, srcImage->xmp.data, srcImage->xmp.size)); if ((planes & AVIF_PLANES_YUV) && srcImage->yuvPlanes[AVIF_CHAN_Y]) { if ((srcImage->yuvFormat != AVIF_PIXEL_FORMAT_YUV400) && @@ -295,14 +295,14 @@ void avifImageDestroy(avifImage * image) avifFree(image); } -void avifImageSetProfileICC(avifImage * image, const uint8_t * icc, size_t iccSize) +avifResult avifImageSetProfileICC(avifImage * image, const uint8_t * icc, size_t iccSize) { - avifRWDataSet(&image->icc, icc, iccSize); + return avifRWDataSet(&image->icc, icc, iccSize); } -void avifImageSetMetadataXMP(avifImage * image, const uint8_t * xmp, size_t xmpSize) +avifResult avifImageSetMetadataXMP(avifImage * image, const uint8_t * xmp, size_t xmpSize) { - avifRWDataSet(&image->xmp, xmp, xmpSize); + return avifRWDataSet(&image->xmp, xmp, xmpSize); } avifResult avifImageAllocatePlanes(avifImage * image, avifPlanesFlags planes) diff --git a/src/codec_aom.c b/src/codec_aom.c index 8f8cef5616..6bf2ea7ccc 100644 --- a/src/codec_aom.c +++ b/src/codec_aom.c @@ -1142,7 +1142,8 @@ static avifResult aomCodecEncodeImage(avifCodec * codec, break; } if (pkt->kind == AOM_CODEC_CX_FRAME_PKT) { - avifCodecEncodeOutputAddSample(output, pkt->data.frame.buf, pkt->data.frame.sz, (pkt->data.frame.flags & AOM_FRAME_IS_KEY)); + AVIF_CHECKRES( + avifCodecEncodeOutputAddSample(output, pkt->data.frame.buf, pkt->data.frame.sz, (pkt->data.frame.flags & AOM_FRAME_IS_KEY))); } } @@ -1187,7 +1188,14 @@ static avifBool aomCodecEncodeFinish(avifCodec * codec, avifCodecEncodeOutput * } if (pkt->kind == AOM_CODEC_CX_FRAME_PKT) { gotPacket = AVIF_TRUE; - avifCodecEncodeOutputAddSample(output, pkt->data.frame.buf, pkt->data.frame.sz, (pkt->data.frame.flags & AOM_FRAME_IS_KEY)); + const avifResult result = avifCodecEncodeOutputAddSample(output, + pkt->data.frame.buf, + pkt->data.frame.sz, + (pkt->data.frame.flags & AOM_FRAME_IS_KEY)); + if (result != AVIF_RESULT_OK) { + avifDiagnosticsPrintf(codec->diag, "avifCodecEncodeOutputAddSample() failed: %s", avifResultToString(result)); + return AVIF_FALSE; + } } } diff --git a/src/codec_avm.c b/src/codec_avm.c index a2ac97deee..8f41bb0d44 100644 --- a/src/codec_avm.c +++ b/src/codec_avm.c @@ -1014,7 +1014,8 @@ static avifResult avmCodecEncodeImage(avifCodec * codec, break; } if (pkt->kind == AOM_CODEC_CX_FRAME_PKT) { - avifCodecEncodeOutputAddSample(output, pkt->data.frame.buf, pkt->data.frame.sz, (pkt->data.frame.flags & AOM_FRAME_IS_KEY)); + AVIF_CHECKRES( + avifCodecEncodeOutputAddSample(output, pkt->data.frame.buf, pkt->data.frame.sz, (pkt->data.frame.flags & AOM_FRAME_IS_KEY))); } } @@ -1059,7 +1060,14 @@ static avifBool avmCodecEncodeFinish(avifCodec * codec, avifCodecEncodeOutput * } if (pkt->kind == AOM_CODEC_CX_FRAME_PKT) { gotPacket = AVIF_TRUE; - avifCodecEncodeOutputAddSample(output, pkt->data.frame.buf, pkt->data.frame.sz, (pkt->data.frame.flags & AOM_FRAME_IS_KEY)); + const avifResult result = avifCodecEncodeOutputAddSample(output, + pkt->data.frame.buf, + pkt->data.frame.sz, + (pkt->data.frame.flags & AOM_FRAME_IS_KEY)); + if (result != AVIF_RESULT_OK) { + avifDiagnosticsPrintf(codec->diag, "avifCodecEncodeOutputAddSample() failed: %s", avifResultToString(result)); + return AVIF_FALSE; + } } } diff --git a/src/codec_rav1e.c b/src/codec_rav1e.c index 28c5999aa2..91dbad211a 100644 --- a/src/codec_rav1e.c +++ b/src/codec_rav1e.c @@ -229,7 +229,10 @@ static avifResult rav1eCodecEncodeImage(avifCodec * codec, goto cleanup; } else if (pkt) { if (pkt->data && (pkt->len > 0)) { - avifCodecEncodeOutputAddSample(output, pkt->data, pkt->len, (pkt->frame_type == RA_FRAME_TYPE_KEY)); + result = avifCodecEncodeOutputAddSample(output, pkt->data, pkt->len, (pkt->frame_type == RA_FRAME_TYPE_KEY)); + if (result != AVIF_RESULT_OK) { + goto cleanup; + } } rav1e_packet_unref(pkt); pkt = NULL; @@ -271,7 +274,10 @@ static avifBool rav1eCodecEncodeFinish(avifCodec * codec, avifCodecEncodeOutput if (pkt) { gotPacket = AVIF_TRUE; if (pkt->data && (pkt->len > 0)) { - avifCodecEncodeOutputAddSample(output, pkt->data, pkt->len, (pkt->frame_type == RA_FRAME_TYPE_KEY)); + if (avifCodecEncodeOutputAddSample(output, pkt->data, pkt->len, (pkt->frame_type == RA_FRAME_TYPE_KEY)) != + AVIF_RESULT_OK) { + return AVIF_FALSE; + } } rav1e_packet_unref(pkt); pkt = NULL; diff --git a/src/codec_svt.c b/src/codec_svt.c index 609f35b597..059adc14b2 100644 --- a/src/codec_svt.c +++ b/src/codec_svt.c @@ -321,10 +321,14 @@ static avifResult dequeue_frame(avifCodec * codec, avifCodecEncodeOutput * outpu if (output_buf != NULL) { encode_at_eos = ((output_buf->flags & EB_BUFFERFLAG_EOS) == EB_BUFFERFLAG_EOS); if (output_buf->p_buffer && (output_buf->n_filled_len > 0)) { - avifCodecEncodeOutputAddSample(output, - output_buf->p_buffer, - output_buf->n_filled_len, - (output_buf->pic_type == EB_AV1_KEY_PICTURE)); + const avifResult result = avifCodecEncodeOutputAddSample(output, + output_buf->p_buffer, + output_buf->n_filled_len, + (output_buf->pic_type == EB_AV1_KEY_PICTURE)); + if (result != AVIF_RESULT_OK) { + svt_av1_enc_release_out_buffer(&output_buf); + return result; + } } svt_av1_enc_release_out_buffer(&output_buf); } diff --git a/src/exif.c b/src/exif.c index 82bf6f38f5..e0100388fa 100644 --- a/src/exif.c +++ b/src/exif.c @@ -142,9 +142,11 @@ avifResult avifImageExtractExifOrientationToIrotImir(avifImage * image) return AVIF_RESULT_OK; } -void avifImageSetMetadataExif(avifImage * image, const uint8_t * exif, size_t exifSize) +avifResult avifImageSetMetadataExif(avifImage * image, const uint8_t * exif, size_t exifSize) { - avifRWDataSet(&image->exif, exif, exifSize); + AVIF_CHECKRES(avifRWDataSet(&image->exif, exif, exifSize)); // Ignore any Exif parsing failure. + // TODO(wtc): Decide whether to ignore or return Exif parsing failures. (void)avifImageExtractExifOrientationToIrotImir(image); + return AVIF_RESULT_OK; } diff --git a/src/io.c b/src/io.c index 892726f6a0..99d87afb8a 100644 --- a/src/io.c +++ b/src/io.c @@ -103,7 +103,7 @@ static avifResult avifIOFileReaderRead(struct avifIO * io, uint32_t readFlags, u return AVIF_RESULT_IO_ERROR; } if (reader->buffer.size < size) { - avifRWDataRealloc(&reader->buffer, size); + AVIF_CHECKRES(avifRWDataRealloc(&reader->buffer, size)); } if (fseek(reader->f, (long)offset, SEEK_SET) != 0) { return AVIF_RESULT_IO_ERROR; @@ -146,12 +146,20 @@ avifIO * avifIOCreateFileReader(const char * filename) fseek(f, 0, SEEK_SET); avifIOFileReader * reader = avifAlloc(sizeof(avifIOFileReader)); + if (!reader) { + fclose(f); + return NULL; + } memset(reader, 0, sizeof(avifIOFileReader)); reader->f = f; reader->io.destroy = avifIOFileReaderDestroy; reader->io.read = avifIOFileReaderRead; reader->io.sizeHint = (uint64_t)fileSize; reader->io.persistent = AVIF_FALSE; - avifRWDataRealloc(&reader->buffer, 1024); + if (avifRWDataRealloc(&reader->buffer, 1024) != AVIF_RESULT_OK) { + avifFree(reader); + fclose(f); + return NULL; + } return (avifIO *)reader; } diff --git a/src/mem.c b/src/mem.c index 5022d26c48..f26891a13d 100644 --- a/src/mem.c +++ b/src/mem.c @@ -9,6 +9,7 @@ void * avifAlloc(size_t size) { void * out = malloc(size); if (out == NULL) { + // TODO(issue #820): Remove once all calling sites propagate the error as AVIF_RESULT_OUT_OF_MEMORY. abort(); } return out; diff --git a/src/rawdata.c b/src/rawdata.c index 248940f3ca..e4653160c8 100644 --- a/src/rawdata.c +++ b/src/rawdata.c @@ -1,33 +1,34 @@ // Copyright 2019 Joe Drago. All rights reserved. // SPDX-License-Identifier: BSD-2-Clause -#include "avif/avif.h" +#include "avif/internal.h" #include -void avifRWDataRealloc(avifRWData * raw, size_t newSize) +avifResult avifRWDataRealloc(avifRWData * raw, size_t newSize) { if (raw->size != newSize) { - uint8_t * old = raw->data; - size_t oldSize = raw->size; - raw->data = avifAlloc(newSize); - raw->size = newSize; - if (oldSize) { - size_t bytesToCopy = (oldSize < raw->size) ? oldSize : raw->size; - memcpy(raw->data, old, bytesToCopy); - avifFree(old); + uint8_t * newData = avifAlloc(newSize); + AVIF_CHECKERR(newData, AVIF_RESULT_OUT_OF_MEMORY); + if (raw->size && newSize) { + memcpy(newData, raw->data, AVIF_MIN(raw->size, newSize)); } + avifFree(raw->data); + raw->data = newData; + raw->size = newSize; } + return AVIF_RESULT_OK; } -void avifRWDataSet(avifRWData * raw, const uint8_t * data, size_t len) +avifResult avifRWDataSet(avifRWData * raw, const uint8_t * data, size_t len) { if (len) { - avifRWDataRealloc(raw, len); + AVIF_CHECKRES(avifRWDataRealloc(raw, len)); memcpy(raw->data, data, len); } else { avifRWDataFree(raw); } + return AVIF_RESULT_OK; } void avifRWDataFree(avifRWData * raw) diff --git a/src/read.c b/src/read.c index e4021898eb..c1f223d53b 100644 --- a/src/read.c +++ b/src/read.c @@ -1243,7 +1243,7 @@ static avifResult avifDecoderItemRead(avifDecoderItem * item, // holding the address of the previous allocation of this buffer). This strategy avoids // use-after-free issues in the AV1 decoder and unnecessary reallocs as a typical // progressive decode use case will eventually decode the final layer anyway. - avifRWDataRealloc(&item->mergedExtents, item->size); + AVIF_CHECKRES(avifRWDataRealloc(&item->mergedExtents, item->size)); item->ownsMergedExtents = AVIF_TRUE; } @@ -1575,7 +1575,7 @@ static avifResult avifDecoderFindMetadata(avifDecoder * decoder, avifMeta * meta AVIF_CHECKERR(avifROStreamReadU32(&exifBoxStream, &exifTiffHeaderOffset), AVIF_RESULT_BMFF_PARSE_FAILED); // unsigned int(32) exif_tiff_header_offset; - avifRWDataSet(&image->exif, avifROStreamCurrent(&exifBoxStream), avifROStreamRemainingBytes(&exifBoxStream)); + AVIF_CHECKRES(avifRWDataSet(&image->exif, avifROStreamCurrent(&exifBoxStream), avifROStreamRemainingBytes(&exifBoxStream))); } else if (!decoder->ignoreXMP && !memcmp(item->type, "mime", 4) && !memcmp(item->contentType.contentType, xmpContentType, xmpContentTypeSize)) { avifROData xmpContents; @@ -1584,7 +1584,7 @@ static avifResult avifDecoderFindMetadata(avifDecoder * decoder, avifMeta * meta return readResult; } - avifImageSetMetadataXMP(image, xmpContents.data, xmpContents.size); + AVIF_CHECKRES(avifImageSetMetadataXMP(image, xmpContents.data, xmpContents.size)); } } return AVIF_RESULT_OK; @@ -2276,7 +2276,9 @@ static avifBool avifParseItemDataBox(avifMeta * meta, const uint8_t * raw, size_ return AVIF_FALSE; } - avifRWDataSet(&meta->idat, raw, rawLen); + if (avifRWDataSet(&meta->idat, raw, rawLen) != AVIF_RESULT_OK) { + return AVIF_FALSE; + } return AVIF_TRUE; } @@ -3398,7 +3400,7 @@ static avifResult avifDecoderPrepareSample(avifDecoder * decoder, avifDecodeSamp if (decoder->io->persistent) { sample->data = sampleContents; } else { - avifRWDataSet((avifRWData *)&sample->data, sampleContents.data, sampleContents.size); + AVIF_CHECKRES(avifRWDataSet((avifRWData *)&sample->data, sampleContents.data, sampleContents.size)); } } } @@ -4031,7 +4033,7 @@ avifResult avifDecoderReset(avifDecoder * decoder) return readResult; } colrICCSeen = AVIF_TRUE; - avifImageSetProfileICC(decoder->image, icc.data, icc.size); + AVIF_CHECKRES(avifImageSetProfileICC(decoder->image, icc.data, icc.size)); } if (prop->u.colr.hasNCLX) { if (colrNCLXSeen) { diff --git a/src/stream.c b/src/stream.c index a5935b4f6d..a4e9d39bdd 100644 --- a/src/stream.c +++ b/src/stream.c @@ -6,6 +6,7 @@ #include #include #include +#include #include // --------------------------------------------------------------------------- @@ -239,8 +240,9 @@ static void makeRoom(avifRWStream * stream, size_t size) while (newSize < neededSize) { newSize += AVIF_STREAM_BUFFER_INCREMENT; } - if (stream->raw->size != newSize) { - avifRWDataRealloc(stream->raw, newSize); + if (avifRWDataRealloc(stream->raw, newSize) != AVIF_RESULT_OK) { + // TODO(issue #820): Return AVIF_RESULT_OUT_OF_MEMORY instead. + abort(); } } diff --git a/src/write.c b/src/write.c index b2b546a48a..0f757b22fa 100644 --- a/src/write.c +++ b/src/write.c @@ -128,11 +128,17 @@ avifCodecEncodeOutput * avifCodecEncodeOutputCreate(void) return NULL; } -void avifCodecEncodeOutputAddSample(avifCodecEncodeOutput * encodeOutput, const uint8_t * data, size_t len, avifBool sync) +avifResult avifCodecEncodeOutputAddSample(avifCodecEncodeOutput * encodeOutput, const uint8_t * data, size_t len, avifBool sync) { avifEncodeSample * sample = (avifEncodeSample *)avifArrayPushPtr(&encodeOutput->samples); - avifRWDataSet(&sample->data, data, len); + AVIF_CHECKERR(sample, AVIF_RESULT_OUT_OF_MEMORY); + const avifResult result = avifRWDataSet(&sample->data, data, len); + if (result != AVIF_RESULT_OK) { + avifArrayPop(&encodeOutput->samples); + return result; + } sample->sync = sync; + return AVIF_RESULT_OK; } void avifCodecEncodeOutputDestroy(avifCodecEncodeOutput * encodeOutput) @@ -345,7 +351,9 @@ static avifItemPropertyDedup * avifItemPropertyDedupCreate(void) if (!avifArrayCreate(&dedup->properties, sizeof(avifItemProperty), 8)) { goto error; } - avifRWDataRealloc(&dedup->buffer, 2048); // This will resize automatically (if necessary) + if (avifRWDataRealloc(&dedup->buffer, 2048) != AVIF_RESULT_OK) { + goto error; + } return dedup; error: @@ -776,7 +784,7 @@ static avifResult avifEncoderDataCreateExifItem(avifEncoderData * data, const av exifItem->irefType = "cdsc"; const uint32_t offset32bit = avifHTONL((uint32_t)exifTiffHeaderOffset); - avifRWDataRealloc(&exifItem->metadataPayload, sizeof(offset32bit) + exif->size); + AVIF_CHECKRES(avifRWDataRealloc(&exifItem->metadataPayload, sizeof(offset32bit) + exif->size)); memcpy(exifItem->metadataPayload.data, &offset32bit, sizeof(offset32bit)); memcpy(exifItem->metadataPayload.data + sizeof(offset32bit), exif->data, exif->size); return AVIF_RESULT_OK; @@ -793,7 +801,7 @@ static avifResult avifEncoderDataCreateXMPItem(avifEncoderData * data, const avi xmpItem->infeContentType = xmpContentType; xmpItem->infeContentTypeSize = xmpContentTypeSize; - avifRWDataSet(&xmpItem->metadataPayload, xmp->data, xmp->size); + AVIF_CHECKRES(avifRWDataSet(&xmpItem->metadataPayload, xmp->data, xmp->size)); return AVIF_RESULT_OK; } diff --git a/tests/aviftest.c b/tests/aviftest.c index 91784cc298..1bac2c7406 100644 --- a/tests/aviftest.c +++ b/tests/aviftest.c @@ -187,7 +187,8 @@ static int runIOTests(const char * dataDir) size_t filenameLen = strlen(filename); if ((ioDirLen + filenameLen) > FILENAME_MAX_LENGTH) { printf("Path too long: %s\n", filename); - return 1; + retCode = 1; + break; } strcpy(fullFilename, ioDir); strcat(fullFilename, filename); @@ -195,16 +196,23 @@ static int runIOTests(const char * dataDir) FILE * f = fopen(fullFilename, "rb"); if (!f) { printf("Can't open for read: %s\n", filename); - return 1; + retCode = 1; + break; } fseek(f, 0, SEEK_END); size_t fileSize = ftell(f); fseek(f, 0, SEEK_SET); - avifRWDataRealloc(&fileBuffer, fileSize); + if (avifRWDataRealloc(&fileBuffer, fileSize) != AVIF_RESULT_OK) { + printf("Out of memory when allocating buffer to read file: %s\n", filename); + fclose(f); + retCode = 1; + break; + } if (fread(fileBuffer.data, 1, fileSize, f) != fileSize) { printf("Can't read entire file: %s\n", filename); fclose(f); - return 1; + retCode = 1; + break; } fclose(f); diff --git a/tests/gtest/avifincrtest.cc b/tests/gtest/avifincrtest.cc index 0a01ebd3cd..ed88a422f8 100644 --- a/tests/gtest/avifincrtest.cc +++ b/tests/gtest/avifincrtest.cc @@ -28,8 +28,10 @@ testutil::AvifRwData ReadFile(const char* file_name) { std::ifstream file(std::string(data_path) + "/" + file_name, std::ios::binary | std::ios::ate); testutil::AvifRwData bytes; - avifRWDataRealloc(&bytes, - file.good() ? static_cast(file.tellg()) : 0); + if (avifRWDataRealloc(&bytes, file.good() ? static_cast(file.tellg()) + : 0) != AVIF_RESULT_OK) { + return {}; + } file.seekg(0, std::ios::beg); file.read(reinterpret_cast(bytes.data), static_cast(bytes.size)); diff --git a/tests/gtest/avifmetadatatest.cc b/tests/gtest/avifmetadatatest.cc index bb6c7b729d..20da875b25 100644 --- a/tests/gtest/avifmetadatatest.cc +++ b/tests/gtest/avifmetadatatest.cc @@ -61,14 +61,17 @@ TEST_P(AvifMetadataTest, EncodeDecode) { ASSERT_NE(image, nullptr); testutil::FillImageGradient(image.get()); // The pixels do not matter. if (use_icc) { - avifImageSetProfileICC(image.get(), kSampleIcc.data(), kSampleIcc.size()); + ASSERT_EQ(avifImageSetProfileICC(image.get(), kSampleIcc.data(), + kSampleIcc.size()), + AVIF_RESULT_OK); } if (use_exif) { const avifTransformFlags old_transform_flags = image->transformFlags; const uint8_t old_irot_angle = image->irot.angle; const uint8_t old_imir_mode = image->imir.mode; - avifImageSetMetadataExif(image.get(), kSampleExif.data(), - kSampleExif.size()); + ASSERT_EQ(avifImageSetMetadataExif(image.get(), kSampleExif.data(), + kSampleExif.size()), + AVIF_RESULT_OK); // kSampleExif is not a valid Exif payload, just some part of it. These // fields should not be modified. EXPECT_EQ(image->transformFlags, old_transform_flags); @@ -76,7 +79,9 @@ TEST_P(AvifMetadataTest, EncodeDecode) { EXPECT_EQ(image->imir.mode, old_imir_mode); } if (use_xmp) { - avifImageSetMetadataXMP(image.get(), kSampleXmp.data(), kSampleXmp.size()); + ASSERT_EQ(avifImageSetMetadataXMP(image.get(), kSampleXmp.data(), + kSampleXmp.size()), + AVIF_RESULT_OK); } // Encode. @@ -334,7 +339,8 @@ TEST(MetadataTest, RotatedJpegBecauseOfIrotImir) { const testutil::AvifImagePtr image = testutil::ReadImage(data_path, "paris_exif_orientation_5.jpg"); ASSERT_NE(image, nullptr); - avifImageSetMetadataExif(image.get(), nullptr, 0); // Clear Exif. + EXPECT_EQ(avifImageSetMetadataExif(image.get(), nullptr, 0), + AVIF_RESULT_OK); // Clear Exif. // Orientation is kept in irot/imir. EXPECT_EQ(image->transformFlags & (AVIF_TRANSFORM_IROT | AVIF_TRANSFORM_IMIR), avifTransformFlags{AVIF_TRANSFORM_IROT | AVIF_TRANSFORM_IMIR}); @@ -367,14 +373,10 @@ TEST(MetadataTest, ExifIfdOffsetLoopingTo8) { // avifImageExtractExifOrientationToIrotImir() internally. // The avifImageExtractExifOrientationToIrotImir() call should not enter an // infinite loop. - // - // TODO(wtc): When we change avifImageSetMetadataExif() to return avifResult, - // assert that the avifImageSetMetadataExif() call returns AVIF_RESULT_OK - // because avifImageExtractExifOrientationToIrotImir() does not verify the - // whole payload, only the parts necessary to extract Exif orientation. - avifImageSetMetadataExif( - image.get(), kBadExifPayload, - sizeof(kBadExifPayload) / sizeof(kBadExifPayload[0])); + ASSERT_EQ(avifImageSetMetadataExif( + image.get(), kBadExifPayload, + sizeof(kBadExifPayload) / sizeof(kBadExifPayload[0])), + AVIF_RESULT_OK); } //------------------------------------------------------------------------------ @@ -425,7 +427,7 @@ TEST(MetadataTest, XMPWithTrailingNullCharacter) { ASSERT_EQ(std::memchr(jpg->xmp.data, '\0', jpg->xmp.size), nullptr); // Append a zero byte to see what happens when encoded with libpng. - avifRWDataRealloc(&jpg->xmp, jpg->xmp.size + 1); + ASSERT_EQ(avifRWDataRealloc(&jpg->xmp, jpg->xmp.size + 1), AVIF_RESULT_OK); jpg->xmp.data[jpg->xmp.size - 1] = '\0'; testutil::WriteImage(jpg.get(), (testing::TempDir() + "xmp_trailing_null.png").c_str());