Skip to content

Commit 364770b

Browse files
Tom94Tom94
committed
feat(jpg): avoid UB pointer casts during load
1 parent 2c57159 commit 364770b

2 files changed

Lines changed: 68 additions & 39 deletions

File tree

‎src/imageio/JpegTurboImageLoader.cpp‎

Lines changed: 34 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -219,25 +219,36 @@ Task<vector<ImageData>>
219219
tlog::debug() << fmt::format("JPEG image info: size={} numChannels={} precision={}", size, numChannels, cinfo.data_precision);
220220

221221
// Allocate memory for image data
222-
const auto numPixels = static_cast<size_t>(size.x()) * size.y();
223-
const auto bytesPerSample = nBytes(pixelFormat);
224-
auto imageData = PixelBuffer::alloc(numPixels * numChannels, pixelFormat);
225-
226-
const auto numBytesPerPixel = numChannels * bytesPerSample;
222+
const auto numPixels = (size_t)size.x() * size.y();
223+
auto buf = PixelBuffer::alloc(numPixels * numChannels, pixelFormat);
227224

228225
// Create row pointers for libjpeg and then read image
229-
HeapArray<JSAMPROW> rowPointers(size.y());
230-
for (int y = 0; y < size.y(); ++y) {
231-
rowPointers[y] = imageData.dataBytes() + y * size.x() * numBytesPerPixel;
232-
}
226+
if (cinfo.data_precision <= 8) {
227+
HeapArray<JSAMPROW> rowPointers(size.y());
228+
for (int y = 0; y < size.y(); ++y) {
229+
rowPointers[y] = buf.data<uint8_t>() + y * size.x() * numChannels;
230+
}
233231

234-
while (cinfo.output_scanline < cinfo.output_height) {
235-
if (cinfo.data_precision <= 8) {
232+
while (cinfo.output_scanline < cinfo.output_height) {
236233
jpeg_read_scanlines(&cinfo, &rowPointers[cinfo.output_scanline], cinfo.output_height - cinfo.output_scanline);
237-
} else if (cinfo.data_precision <= 12) {
238-
jpeg12_read_scanlines(&cinfo, (J12SAMPARRAY)&rowPointers[cinfo.output_scanline], cinfo.output_height - cinfo.output_scanline);
239-
} else {
240-
jpeg16_read_scanlines(&cinfo, (J16SAMPARRAY)&rowPointers[cinfo.output_scanline], cinfo.output_height - cinfo.output_scanline);
234+
}
235+
} else if (cinfo.data_precision <= 12) {
236+
HeapArray<J12SAMPROW> rowPointers(size.y());
237+
for (int y = 0; y < size.y(); ++y) {
238+
rowPointers[y] = buf.data<int16_t>() + y * size.x() * numChannels;
239+
}
240+
241+
while (cinfo.output_scanline < cinfo.output_height) {
242+
jpeg12_read_scanlines(&cinfo, &rowPointers[cinfo.output_scanline], cinfo.output_height - cinfo.output_scanline);
243+
}
244+
} else {
245+
HeapArray<J16SAMPROW> rowPointers(size.y());
246+
for (int y = 0; y < size.y(); ++y) {
247+
rowPointers[y] = buf.data<uint16_t>() + y * size.x() * numChannels;
248+
}
249+
250+
while (cinfo.output_scanline < cinfo.output_height) {
251+
jpeg16_read_scanlines(&cinfo, &rowPointers[cinfo.output_scanline], cinfo.output_height - cinfo.output_scanline);
241252
}
242253
}
243254

@@ -421,7 +432,7 @@ Task<vector<ImageData>>
421432
const EOrientation exifOrientation = exif.getOrientation();
422433
if (exifOrientation != EOrientation::None) {
423434
orientation = exifOrientation;
424-
tlog::debug() << fmt::format("EXIF image orientation: {}", (int)orientation);
435+
tlog::debug() << fmt::format("EXIF image orientation: {}", toString(orientation));
425436
}
426437

427438
imageInfo.appleMakerNoteIfd = exif.tryGetAppleMakerNote();
@@ -441,7 +452,7 @@ Task<vector<ImageData>>
441452
const EOrientation xmpOrientation = xmp.orientation();
442453
if (xmpOrientation != EOrientation::None) {
443454
orientation = xmpOrientation;
444-
tlog::debug() << fmt::format("XMP image orientation: {}", (int)orientation);
455+
tlog::debug() << fmt::format("XMP image orientation: {}", toString(orientation));
445456
}
446457

447458
isoGainmapMetadata = xmp.isoGainMapMetadata();
@@ -458,7 +469,7 @@ Task<vector<ImageData>>
458469
}
459470

460471
if (orientation != EOrientation::None) {
461-
size = co_await orientToTopLeft(imageData, size, orientation, priority);
472+
size = co_await orientToTopLeft(buf, size, orientation, priority);
462473
}
463474

464475
if (!appN.iso.empty()) {
@@ -511,11 +522,13 @@ Task<vector<ImageData>>
511522

512523
const auto jpegDataToFloat32 = [&](bool fromSrgb, const MultiChannelView<float>& dst) -> Task<void> {
513524
if (pixelFormat == EPixelFormat::U8) {
514-
co_await jpegDataToFloat32Typed(imageData.data<const uint8_t>(), fromSrgb, dst);
525+
co_await jpegDataToFloat32Typed(buf.data<const uint8_t>(), fromSrgb, dst);
526+
} else if (pixelFormat == EPixelFormat::I16) {
527+
co_await jpegDataToFloat32Typed(buf.data<const int16_t>(), fromSrgb, dst);
515528
} else if (pixelFormat == EPixelFormat::U16) {
516-
co_await jpegDataToFloat32Typed(imageData.data<const uint16_t>(), fromSrgb, dst);
529+
co_await jpegDataToFloat32Typed(buf.data<const uint16_t>(), fromSrgb, dst);
517530
} else {
518-
throw ImageLoadError{fmt::format("Unsupported pixel format: {}", (int)pixelFormat)};
531+
throw ImageLoadError{fmt::format("Unsupported pixel format: {}", toString(pixelFormat))};
519532
}
520533
};
521534

‎src/imageio/TiffImageLoader.cpp‎

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1578,6 +1578,8 @@ Task<ImageData> decodeJpeg(
15781578
throw ImageLoadError{fmt::format("Unsupported JPEG precision: {} bits per sample.", precision)};
15791579
}
15801580

1581+
const auto pixelFormat = cinfo.data_precision > 8 ? cinfo.data_precision > 12 ? EPixelFormat::U16 : EPixelFormat::I16 : EPixelFormat::U8;
1582+
15811583
// Suppress all color conversion; output in the native colorspace. We'll convert outselves.
15821584
cinfo.out_color_space = cinfo.jpeg_color_space;
15831585
cinfo.quantize_colors = false;
@@ -1618,31 +1620,45 @@ Task<ImageData> decodeJpeg(
16181620
// (int)cinfo.jpeg_color_space
16191621
// );
16201622

1621-
const auto numBytesPerSample = divRoundUp(precision, 8);
1622-
const auto rowStride = (size_t)width * numComponents * numBytesPerSample;
1623-
HeapArray<uint8_t> buf((size_t)width * height * numComponents * numBytesPerSample);
1623+
auto buf = PixelBuffer::alloc((size_t)width * height * numComponents, pixelFormat);
16241624

1625-
HeapArray<JSAMPROW> rowPointers(height);
1626-
for (size_t y = 0; y < height; ++y) {
1627-
rowPointers[y] = &buf[y * rowStride];
1628-
}
1625+
if (cinfo.data_precision <= 8) {
1626+
HeapArray<JSAMPROW> rowPointers(height);
1627+
for (size_t y = 0; y < height; ++y) {
1628+
rowPointers[y] = buf.data<uint8_t>() + y * width * numComponents;
1629+
}
16291630

1630-
while (cinfo.output_scanline < cinfo.output_height) {
1631-
if (precision <= 8) {
1631+
while (cinfo.output_scanline < cinfo.output_height) {
16321632
jpeg_read_scanlines(&cinfo, &rowPointers[cinfo.output_scanline], cinfo.output_height - cinfo.output_scanline);
1633-
} else if (precision <= 12) {
1634-
jpeg12_read_scanlines(&cinfo, (J12SAMPARRAY)&rowPointers[cinfo.output_scanline], cinfo.output_height - cinfo.output_scanline);
1635-
} else {
1636-
jpeg16_read_scanlines(&cinfo, (J16SAMPARRAY)&rowPointers[cinfo.output_scanline], cinfo.output_height - cinfo.output_scanline);
1633+
}
1634+
} else if (cinfo.data_precision <= 12) {
1635+
HeapArray<J12SAMPROW> rowPointers(height);
1636+
for (size_t y = 0; y < height; ++y) {
1637+
rowPointers[y] = buf.data<int16_t>() + y * width * numComponents;
1638+
}
1639+
1640+
while (cinfo.output_scanline < cinfo.output_height) {
1641+
jpeg12_read_scanlines(&cinfo, &rowPointers[cinfo.output_scanline], cinfo.output_height - cinfo.output_scanline);
1642+
}
1643+
} else {
1644+
HeapArray<J16SAMPROW> rowPointers(height);
1645+
for (size_t y = 0; y < height; ++y) {
1646+
rowPointers[y] = buf.data<uint16_t>() + y * width * numComponents;
1647+
}
1648+
1649+
while (cinfo.output_scanline < cinfo.output_height) {
1650+
jpeg16_read_scanlines(&cinfo, &rowPointers[cinfo.output_scanline], cinfo.output_height - cinfo.output_scanline);
16371651
}
16381652
}
16391653

1640-
if (numBytesPerSample == 1) {
1641-
co_await toFloat32(buf.data(), tileNumComponents, outView, false, priority, scale);
1642-
} else if (numBytesPerSample == 2) {
1643-
co_await toFloat32((const uint16_t*)buf.data(), tileNumComponents, outView, false, priority, scale);
1654+
if (pixelFormat == EPixelFormat::U8) {
1655+
co_await toFloat32(buf.data<const uint8_t>(), tileNumComponents, outView, false, priority, scale);
1656+
} else if (pixelFormat == EPixelFormat::I16) {
1657+
co_await toFloat32(buf.data<const int16_t>(), tileNumComponents, outView, false, priority, scale);
1658+
} else if (pixelFormat == EPixelFormat::U16) {
1659+
co_await toFloat32(buf.data<const uint16_t>(), tileNumComponents, outView, false, priority, scale);
16441660
} else {
1645-
throw ImageLoadError{fmt::format("Unsupported number of bytes per sample: {}", numBytesPerSample)};
1661+
throw ImageLoadError{fmt::format("Unsupported pixel format: {}", toString(pixelFormat))};
16461662
}
16471663

16481664
decompressGuard.disarm();

0 commit comments

Comments
 (0)