Skip to content

Fix HEIF/AVIF output buffer overflow #448

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

Open
gregbenz wants to merge 1 commit into
base: main
Choose a base branch
from
+130 −32
Open

Fix HEIF/AVIF output buffer overflow #448

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
61 changes: 45 additions & 16 deletions lib/src/avifultrahdr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#ifdef UHDR_ENABLE_HEIF

#include <cstdlib>
#include <limits>
#include <map>

#include "ultrahdr/avifultrahdr.h"
Expand All @@ -37,19 +38,30 @@ class MemoryWriter {

size_t size() const { return size_; }

void write(const void* data, size_t size) {
if (capacity_ - size_ < size) {
size_t new_capacity = capacity_ + size;
uint8_t* new_data = static_cast<uint8_t*>(malloc(new_capacity));
if (data_) {
memcpy(new_data, data_, size_);
free(data_);
struct heif_error write(const void* data, size_t size) {
if (size == 0) return {heif_error_Ok, heif_suberror_Unspecified, nullptr};
if (data == nullptr) {
return {heif_error_Usage_error, heif_suberror_Null_pointer_argument,
"output writer received null data"};
}
if (size > std::numeric_limits<size_t>::max() - size_) {
return {heif_error_Memory_allocation_error, heif_suberror_Unspecified,
"output size overflow"};
}

const size_t new_size = size_ + size;
if (new_size > capacity_) {
uint8_t* new_data = static_cast<uint8_t*>(realloc(data_, new_size));
if (new_data == nullptr) {
return {heif_error_Memory_allocation_error, heif_suberror_Unspecified,
"failed to allocate output buffer"};
}
data_ = new_data;
capacity_ = new_capacity;
capacity_ = new_size;
}
memcpy(&data_[size_], data, size);
size_ += size;
size_ = new_size;
return {heif_error_Ok, heif_suberror_Unspecified, nullptr};
}

public:
Expand All @@ -60,12 +72,12 @@ class MemoryWriter {

static struct heif_error writer_write([[maybe_unused]] struct heif_context* ctx, const void* data,
size_t size, void* userdata) {
if (userdata == nullptr) {
return {heif_error_Usage_error, heif_suberror_Null_pointer_argument,
"output writer is null"};
}
MemoryWriter* writer = static_cast<MemoryWriter*>(userdata);
writer->write(data, size);
struct heif_error err {
heif_error_Ok, heif_suberror_Unspecified, nullptr
};
return err;
return writer->write(data, size);
}

static struct heif_error fill_img_plane(heif_image* img, heif_channel channel, void* srcBuffer,
Expand Down Expand Up @@ -381,6 +393,7 @@ uhdr_error_info_t AvifUltraHdr::encodeAvifUltraHdr(uhdr_raw_image_t* sdr_intent,

MemoryWriter writer;
struct heif_writer w;
heif_error write_err;
w.writer_api_version = 1;
w.write = writer_write;

Expand Down Expand Up @@ -524,9 +537,25 @@ uhdr_error_info_t AvifUltraHdr::encodeAvifUltraHdr(uhdr_raw_image_t* sdr_intent,
options, iso_data.data(), iso_data.size(),
hdrNclx, &secondaryHandle));

heif_context_write(ctx, &w, &writer);
write_err = heif_context_write(ctx, &w, &writer);
if (write_err.code != heif_error_Ok) {
status.error_code = write_err.code == heif_error_Memory_allocation_error
? UHDR_CODEC_MEM_ERROR
: UHDR_CODEC_ERROR;
status.has_detail = 1;
snprintf(status.detail, sizeof status.detail, "%s", write_err.message);
goto CleanUp;
}
if (writer.size() > dest->capacity) {
status.error_code = UHDR_CODEC_MEM_ERROR;
status.has_detail = 1;
snprintf(status.detail, sizeof status.detail,
"destination buffer is too small, capacity is %zu, required size is %zu",
dest->capacity, writer.size());
goto CleanUp;
}
memcpy(dest->data, writer.data(), writer.size());
dest->data_sz = dest->capacity = writer.size();
dest->data_sz = writer.size();

CleanUp:
if (baseImage) heif_image_release(baseImage);
Expand Down
61 changes: 45 additions & 16 deletions lib/src/heifultrahdr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#ifdef UHDR_ENABLE_HEIF

#include <cstdlib>
#include <limits>
#include <map>

#include "ultrahdr/heifultrahdr.h"
Expand All @@ -37,19 +38,30 @@ class MemoryWriter {

size_t size() const { return size_; }

void write(const void* data, size_t size) {
if (capacity_ - size_ < size) {
size_t new_capacity = capacity_ + size;
uint8_t* new_data = static_cast<uint8_t*>(malloc(new_capacity));
if (data_) {
memcpy(new_data, data_, size_);
free(data_);
struct heif_error write(const void* data, size_t size) {
if (size == 0) return {heif_error_Ok, heif_suberror_Unspecified, nullptr};
if (data == nullptr) {
return {heif_error_Usage_error, heif_suberror_Null_pointer_argument,
"output writer received null data"};
}
if (size > std::numeric_limits<size_t>::max() - size_) {
return {heif_error_Memory_allocation_error, heif_suberror_Unspecified,
"output size overflow"};
}

const size_t new_size = size_ + size;
if (new_size > capacity_) {
uint8_t* new_data = static_cast<uint8_t*>(realloc(data_, new_size));
if (new_data == nullptr) {
return {heif_error_Memory_allocation_error, heif_suberror_Unspecified,
"failed to allocate output buffer"};
}
data_ = new_data;
capacity_ = new_capacity;
capacity_ = new_size;
}
memcpy(&data_[size_], data, size);
size_ += size;
size_ = new_size;
return {heif_error_Ok, heif_suberror_Unspecified, nullptr};
}

public:
Expand All @@ -60,12 +72,12 @@ class MemoryWriter {

static struct heif_error writer_write([[maybe_unused]] struct heif_context* ctx, const void* data,
size_t size, void* userdata) {
if (userdata == nullptr) {
return {heif_error_Usage_error, heif_suberror_Null_pointer_argument,
"output writer is null"};
}
MemoryWriter* writer = static_cast<MemoryWriter*>(userdata);
writer->write(data, size);
struct heif_error err {
heif_error_Ok, heif_suberror_Unspecified, nullptr
};
return err;
return writer->write(data, size);
}

static struct heif_error fill_img_plane(heif_image* img, heif_channel channel, void* srcBuffer,
Expand Down Expand Up @@ -381,6 +393,7 @@ uhdr_error_info_t HeifUltraHdr::encodeHeicUltraHdr(uhdr_raw_image_t* sdr_intent,

MemoryWriter writer;
struct heif_writer w;
heif_error write_err;
w.writer_api_version = 1;
w.write = writer_write;

Expand Down Expand Up @@ -524,9 +537,25 @@ uhdr_error_info_t HeifUltraHdr::encodeHeicUltraHdr(uhdr_raw_image_t* sdr_intent,
options, iso_data.data(), iso_data.size(),
hdrNclx, &secondaryHandle));

heif_context_write(ctx, &w, &writer);
write_err = heif_context_write(ctx, &w, &writer);
if (write_err.code != heif_error_Ok) {
status.error_code = write_err.code == heif_error_Memory_allocation_error
? UHDR_CODEC_MEM_ERROR
: UHDR_CODEC_ERROR;
status.has_detail = 1;
snprintf(status.detail, sizeof status.detail, "%s", write_err.message);
goto CleanUp;
}
if (writer.size() > dest->capacity) {
status.error_code = UHDR_CODEC_MEM_ERROR;
status.has_detail = 1;
snprintf(status.detail, sizeof status.detail,
"destination buffer is too small, capacity is %zu, required size is %zu",
dest->capacity, writer.size());
goto CleanUp;
}
memcpy(dest->data, writer.data(), writer.size());
dest->data_sz = dest->capacity = writer.size();
dest->data_sz = writer.size();

CleanUp:
if (baseImage) heif_image_release(baseImage);
Expand Down
40 changes: 40 additions & 0 deletions tests/ultrahdr_api_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,26 @@ TEST_F(UltraHdrApiTest, HeicEncodeApi1AndDecode) {
uhdr_release_encoder(enc);
}

TEST_F(UltraHdrApiTest, HeicEncodeRejectsUndersizedDestination) {
std::vector<uint8_t> backing_store(6 * kImageWidth * kImageHeight, 0xa5);
uhdr_compressed_image_t dest{};
dest.data = backing_store.data();
dest.capacity = 1;

HeifUltraHdr codec;
uhdr_error_info_t status = codec.encodeHeicUltraHdr(&mHdrRaw, &dest, 85, nullptr);
if (status.error_code != UHDR_CODEC_OK && status.has_detail &&
(strstr(status.detail, "Unsupported file-type") != nullptr ||
strstr(status.detail, "No encoder") != nullptr)) {
GTEST_SKIP() << "HEVC encoder plugin not available in environment: " << status.detail;
}

EXPECT_EQ(status.error_code, UHDR_CODEC_MEM_ERROR);
EXPECT_EQ(dest.capacity, 1u);
EXPECT_EQ(dest.data_sz, 0u);
EXPECT_EQ(backing_store.front(), 0xa5);
}

TEST_F(UltraHdrApiTest, HeicCompressedIntentsUnsupported) {
uhdr_codec_private_t* enc = uhdr_create_encoder();
ASSERT_NE(enc, nullptr);
Expand Down Expand Up @@ -375,6 +395,26 @@ TEST_F(UltraHdrApiTest, AvifEncodeApi1AndDecode) {
uhdr_release_encoder(enc);
}

TEST_F(UltraHdrApiTest, AvifEncodeRejectsUndersizedDestination) {
std::vector<uint8_t> backing_store(6 * kImageWidth * kImageHeight, 0xa5);
uhdr_compressed_image_t dest{};
dest.data = backing_store.data();
dest.capacity = 1;

AvifUltraHdr codec;
uhdr_error_info_t status = codec.encodeAvifUltraHdr(&mHdrRaw, &dest, 85, nullptr);
if (status.error_code != UHDR_CODEC_OK && status.has_detail &&
(strstr(status.detail, "Unsupported file-type") != nullptr ||
strstr(status.detail, "No encoder") != nullptr)) {
GTEST_SKIP() << "AV1 encoder plugin not available in environment: " << status.detail;
}

EXPECT_EQ(status.error_code, UHDR_CODEC_MEM_ERROR);
EXPECT_EQ(dest.capacity, 1u);
EXPECT_EQ(dest.data_sz, 0u);
EXPECT_EQ(backing_store.front(), 0xa5);
}

TEST_F(UltraHdrApiTest, AvifCompressedIntentsUnsupported) {
uhdr_codec_private_t* enc = uhdr_create_encoder();
ASSERT_NE(enc, nullptr);
Expand Down
Loading