Mark unsafe buffer regions in CPDF_DIB and CPDF_TransferFuncDIB.
-- Introduce INCR_ON_APPLE() macro to avoid #ifdef inside an expansion
of the UNSAFE_BUFFERS() macro. The MSVC build breaks if such an
#ifdef occurs.
Bug: 42271175
Change-Id: I100bd8c7ef2c9e8a1d6a68a9b719906842adf7b6
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/119679
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Thomas Sepez <tsepez@google.com>
diff --git a/core/fpdfapi/page/cpdf_dib.cpp b/core/fpdfapi/page/cpdf_dib.cpp
index a6caa68..782a377 100644
--- a/core/fpdfapi/page/cpdf_dib.cpp
+++ b/core/fpdfapi/page/cpdf_dib.cpp
@@ -4,11 +4,6 @@
// Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
-#if defined(UNSAFE_BUFFERS_BUILD)
-// TODO(crbug.com/pdfium/2154): resolve buffer safety issues.
-#pragma allow_unsafe_buffers
-#endif
-
#include "core/fpdfapi/page/cpdf_dib.h"
#include <stdint.h>
@@ -40,6 +35,7 @@
#include "core/fxcodec/scanlinedecoder.h"
#include "core/fxcrt/check.h"
#include "core/fxcrt/check_op.h"
+#include "core/fxcrt/compiler_specific.h"
#include "core/fxcrt/data_vector.h"
#include "core/fxcrt/fx_memcpy_wrappers.h"
#include "core/fxcrt/fx_safe_types.h"
@@ -57,18 +53,20 @@
unsigned int GetBits8(const uint8_t* pData, uint64_t bitpos, size_t nbits) {
DCHECK(nbits == 1 || nbits == 2 || nbits == 4 || nbits == 8 || nbits == 16);
DCHECK_EQ((bitpos & (nbits - 1)), 0u);
- unsigned int byte = pData[bitpos / 8];
- if (nbits == 8)
- return byte;
-
- if (nbits == 16)
- return byte * 256 + pData[bitpos / 8 + 1];
-
- return (byte >> (8 - nbits - (bitpos % 8))) & ((1 << nbits) - 1);
+ UNSAFE_TODO({
+ unsigned int byte = pData[bitpos / 8];
+ if (nbits == 8) {
+ return byte;
+ }
+ if (nbits == 16) {
+ return byte * 256 + pData[bitpos / 8 + 1];
+ }
+ return (byte >> (8 - nbits - (bitpos % 8))) & ((1 << nbits) - 1);
+ });
}
bool GetBitValue(const uint8_t* pSrc, uint32_t pos) {
- return pSrc[pos / 8] & (1 << (7 - pos % 8));
+ return UNSAFE_TODO(pSrc[pos / 8] & (1 << (7 - pos % 8)));
}
// Just to sanity check and filter out obvious bad values.
@@ -87,10 +85,13 @@
bool AreColorIndicesOutOfBounds(const uint8_t* indices,
const DIB_COMP_DATA* comp_data,
size_t count) {
- for (size_t i = 0; i < count; ++i) {
- if (IsColorIndexOutOfBounds(indices[i], comp_data[i]))
- return true;
- }
+ UNSAFE_TODO({
+ for (size_t i = 0; i < count; ++i) {
+ if (IsColorIndexOutOfBounds(indices[i], comp_data[i])) {
+ return true;
+ }
+ }
+ });
return false;
}
@@ -744,30 +745,34 @@
for (uint32_t row = 0; row < image_info.height; ++row) {
const uint8_t* src = result_bitmap->GetScanline(row).data();
uint8_t* dest = rgb_bitmap->GetWritableScanline(row).data();
- for (uint32_t col = 0; col < image_info.width; ++col) {
- uint8_t a = src[3];
- m_JpxInlineData.data.push_back(a);
- uint8_t na = 255 - a;
- uint8_t b = (src[0] * a + 255 * na) / 255;
- uint8_t g = (src[1] * a + 255 * na) / 255;
- uint8_t r = (src[2] * a + 255 * na) / 255;
- dest[0] = b;
- dest[1] = g;
- dest[2] = r;
- src += 4;
- dest += 3;
- }
+ UNSAFE_TODO({
+ for (uint32_t col = 0; col < image_info.width; ++col) {
+ uint8_t a = src[3];
+ m_JpxInlineData.data.push_back(a);
+ uint8_t na = 255 - a;
+ uint8_t b = (src[0] * a + 255 * na) / 255;
+ uint8_t g = (src[1] * a + 255 * na) / 255;
+ uint8_t r = (src[2] * a + 255 * na) / 255;
+ dest[0] = b;
+ dest[1] = g;
+ dest[2] = r;
+ src += 4;
+ dest += 3;
+ }
+ });
}
} else {
// TODO(thestig): Is there existing code that does this already?
for (uint32_t row = 0; row < image_info.height; ++row) {
const uint8_t* src = result_bitmap->GetScanline(row).data();
uint8_t* dest = rgb_bitmap->GetWritableScanline(row).data();
- for (uint32_t col = 0; col < image_info.width; ++col) {
- FXSYS_memcpy(dest, src, 3);
- src += 4;
- dest += 3;
- }
+ UNSAFE_TODO({
+ for (uint32_t col = 0; col < image_info.width; ++col) {
+ FXSYS_memcpy(dest, src, 3);
+ src += 4;
+ dest += 3;
+ }
+ });
}
}
result_bitmap = std::move(rgb_bitmap);
@@ -777,10 +782,12 @@
int scale = 8 - m_bpc;
for (uint32_t row = 0; row < image_info.height; ++row) {
uint8_t* scanline = result_bitmap->GetWritableScanline(row).data();
- for (uint32_t col = 0; col < image_info.width; ++col) {
- *scanline = (*scanline) >> scale;
- ++scanline;
- }
+ UNSAFE_TODO({
+ for (uint32_t col = 0; col < image_info.width; ++col) {
+ *scanline = (*scanline) >> scale;
+ ++scanline;
+ }
+ });
}
}
@@ -1096,40 +1103,46 @@
const uint8_t* src_pos = src_scan.data();
switch (m_bpc) {
case 8:
- for (int column = 0; column < m_Width; column++) {
- *dest_pos++ = src_pos[2];
- *dest_pos++ = src_pos[1];
- *dest_pos++ = *src_pos;
- src_pos += 3;
- }
+ UNSAFE_TODO({
+ for (int column = 0; column < m_Width; column++) {
+ *dest_pos++ = src_pos[2];
+ *dest_pos++ = src_pos[1];
+ *dest_pos++ = *src_pos;
+ src_pos += 3;
+ }
+ });
break;
case 16:
- for (int col = 0; col < m_Width; col++) {
- *dest_pos++ = src_pos[4];
- *dest_pos++ = src_pos[2];
- *dest_pos++ = *src_pos;
- src_pos += 6;
- }
+ UNSAFE_TODO({
+ for (int col = 0; col < m_Width; col++) {
+ *dest_pos++ = src_pos[4];
+ *dest_pos++ = src_pos[2];
+ *dest_pos++ = *src_pos;
+ src_pos += 6;
+ }
+ });
break;
default:
const unsigned int max_data = (1 << m_bpc) - 1;
uint64_t src_bit_pos = 0;
size_t dest_byte_pos = 0;
- for (int column = 0; column < m_Width; column++) {
- unsigned int R = GetBits8(src_scan.data(), src_bit_pos, m_bpc);
- src_bit_pos += m_bpc;
- unsigned int G = GetBits8(src_scan.data(), src_bit_pos, m_bpc);
- src_bit_pos += m_bpc;
- unsigned int B = GetBits8(src_scan.data(), src_bit_pos, m_bpc);
- src_bit_pos += m_bpc;
- R = std::min(R, max_data);
- G = std::min(G, max_data);
- B = std::min(B, max_data);
- dest_pos[dest_byte_pos] = B * 255 / max_data;
- dest_pos[dest_byte_pos + 1] = G * 255 / max_data;
- dest_pos[dest_byte_pos + 2] = R * 255 / max_data;
- dest_byte_pos += 3;
- }
+ UNSAFE_TODO({
+ for (int column = 0; column < m_Width; column++) {
+ unsigned int R = GetBits8(src_scan.data(), src_bit_pos, m_bpc);
+ src_bit_pos += m_bpc;
+ unsigned int G = GetBits8(src_scan.data(), src_bit_pos, m_bpc);
+ src_bit_pos += m_bpc;
+ unsigned int B = GetBits8(src_scan.data(), src_bit_pos, m_bpc);
+ src_bit_pos += m_bpc;
+ R = std::min(R, max_data);
+ G = std::min(G, max_data);
+ B = std::min(B, max_data);
+ dest_pos[dest_byte_pos] = B * 255 / max_data;
+ dest_pos[dest_byte_pos + 1] = G * 255 / max_data;
+ dest_pos[dest_byte_pos + 2] = R * 255 / max_data;
+ dest_byte_pos += 3;
+ }
+ });
break;
}
return true;
@@ -1178,7 +1191,7 @@
if (m_bImageMask && m_bDefaultDecode) {
for (uint32_t i = 0; i < src_pitch_value; i++) {
// TODO(tsepez): Bounds check if cost is acceptable.
- m_LineBuf[i] = ~pSrcLine.data()[i];
+ UNSAFE_TODO(m_LineBuf[i] = ~pSrcLine.data()[i]);
}
return pdfium::make_span(m_LineBuf).first(src_pitch_value);
}
@@ -1190,9 +1203,11 @@
uint32_t reset_argb = Get1BitResetValue();
uint32_t set_argb = Get1BitSetValue();
uint32_t* dest_scan = reinterpret_cast<uint32_t*>(m_MaskBuf.data());
- for (int col = 0; col < m_Width; col++, dest_scan++) {
- *dest_scan = GetBitValue(pSrcLine.data(), col) ? set_argb : reset_argb;
- }
+ UNSAFE_TODO({
+ for (int col = 0; col < m_Width; col++, dest_scan++) {
+ *dest_scan = GetBitValue(pSrcLine.data(), col) ? set_argb : reset_argb;
+ }
+ });
return pdfium::make_span(m_MaskBuf).first(m_Width * sizeof(uint32_t));
}
if (m_bpc * m_nComponents <= 8) {
@@ -1219,35 +1234,40 @@
uint8_t* pDestPixel = m_MaskBuf.data();
const uint8_t* pSrcPixel = m_LineBuf.data();
pdfium::span<const uint32_t> palette = GetPaletteSpan();
- if (HasPalette()) {
- for (int col = 0; col < m_Width; col++) {
- uint8_t index = *pSrcPixel++;
- *pDestPixel++ = FXARGB_B(palette[index]);
- *pDestPixel++ = FXARGB_G(palette[index]);
- *pDestPixel++ = FXARGB_R(palette[index]);
- *pDestPixel++ =
- IsColorIndexOutOfBounds(index, m_CompData[0]) ? 0xFF : 0;
+ UNSAFE_TODO({
+ if (HasPalette()) {
+ for (int col = 0; col < m_Width; col++) {
+ uint8_t index = *pSrcPixel++;
+ *pDestPixel++ = FXARGB_B(palette[index]);
+ *pDestPixel++ = FXARGB_G(palette[index]);
+ *pDestPixel++ = FXARGB_R(palette[index]);
+ *pDestPixel++ =
+ IsColorIndexOutOfBounds(index, m_CompData[0]) ? 0xFF : 0;
+ }
+ } else {
+ for (int col = 0; col < m_Width; col++) {
+ uint8_t index = *pSrcPixel++;
+ *pDestPixel++ = index;
+ *pDestPixel++ = index;
+ *pDestPixel++ = index;
+ *pDestPixel++ =
+ IsColorIndexOutOfBounds(index, m_CompData[0]) ? 0xFF : 0;
+ }
}
- } else {
- for (int col = 0; col < m_Width; col++) {
- uint8_t index = *pSrcPixel++;
- *pDestPixel++ = index;
- *pDestPixel++ = index;
- *pDestPixel++ = index;
- *pDestPixel++ =
- IsColorIndexOutOfBounds(index, m_CompData[0]) ? 0xFF : 0;
- }
- }
+ });
return pdfium::make_span(m_MaskBuf).first(4 * m_Width);
}
if (m_bColorKey) {
if (m_nComponents == 3 && m_bpc == 8) {
- uint8_t* alpha_channel = m_MaskBuf.data() + 3;
- for (int col = 0; col < m_Width; col++) {
- const uint8_t* pPixel = pSrcLine.data() + col * 3;
- alpha_channel[col * 4] =
- AreColorIndicesOutOfBounds(pPixel, m_CompData.data(), 3) ? 0xFF : 0;
- }
+ UNSAFE_TODO({
+ uint8_t* alpha_channel = m_MaskBuf.data() + 3;
+ for (int col = 0; col < m_Width; col++) {
+ const uint8_t* pPixel = pSrcLine.data() + col * 3;
+ alpha_channel[col * 4] =
+ AreColorIndicesOutOfBounds(pPixel, m_CompData.data(), 3) ? 0xFF
+ : 0;
+ }
+ });
} else {
fxcrt::spanset(pdfium::make_span(m_MaskBuf), 0xFF);
}
@@ -1263,12 +1283,14 @@
// TODO(tsepez): Bounds check if cost is acceptable.
const uint8_t* pSrcPixel = pSrcLine.data();
uint8_t* pDestPixel = m_MaskBuf.data();
- for (int col = 0; col < m_Width; col++) {
- *pDestPixel++ = *pSrcPixel++;
- *pDestPixel++ = *pSrcPixel++;
- *pDestPixel++ = *pSrcPixel++;
- pDestPixel++;
- }
+ UNSAFE_TODO({
+ for (int col = 0; col < m_Width; col++) {
+ *pDestPixel++ = *pSrcPixel++;
+ *pDestPixel++ = *pSrcPixel++;
+ *pDestPixel++ = *pSrcPixel++;
+ pDestPixel++;
+ }
+ });
return pdfium::make_span(m_MaskBuf).first(4 * m_Width);
}
diff --git a/core/fpdfapi/page/cpdf_transferfuncdib.cpp b/core/fpdfapi/page/cpdf_transferfuncdib.cpp
index cfc168a..7d21975 100644
--- a/core/fpdfapi/page/cpdf_transferfuncdib.cpp
+++ b/core/fpdfapi/page/cpdf_transferfuncdib.cpp
@@ -4,11 +4,6 @@
// Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
-#if defined(UNSAFE_BUFFERS_BUILD)
-// TODO(crbug.com/pdfium/2154): resolve buffer safety issues.
-#pragma allow_unsafe_buffers
-#endif
-
#include "core/fpdfapi/page/cpdf_transferfuncdib.h"
#include <utility>
@@ -17,8 +12,15 @@
#include "core/fpdfapi/page/cpdf_transferfunc.h"
#include "core/fpdfapi/parser/cpdf_dictionary.h"
#include "core/fxcrt/check.h"
+#include "core/fxcrt/compiler_specific.h"
#include "core/fxge/calculate_pitch.h"
+#if BUILDFLAG(IS_APPLE)
+#define INCR_ON_APPLE(x) ++x
+#else
+#define INCR_ON_APPLE(x)
+#endif
+
CPDF_TransferFuncDIB::CPDF_TransferFuncDIB(
RetainPtr<CFX_DIBBase> pSrc,
RetainPtr<CPDF_TransferFunc> pTransferFunc)
@@ -60,72 +62,78 @@
int g1 = m_RampG[255];
int b1 = m_RampB[255];
int index = 0;
- for (int i = 0; i < m_Width; i++) {
- if (src_buf[i / 8] & (1 << (7 - i % 8))) {
- m_Scanline[index++] = b1;
- m_Scanline[index++] = g1;
- m_Scanline[index++] = r1;
- } else {
- m_Scanline[index++] = b0;
- m_Scanline[index++] = g0;
- m_Scanline[index++] = r0;
+ UNSAFE_TODO({
+ for (int i = 0; i < m_Width; i++) {
+ if (src_buf[i / 8] & (1 << (7 - i % 8))) {
+ m_Scanline[index++] = b1;
+ m_Scanline[index++] = g1;
+ m_Scanline[index++] = r1;
+ } else {
+ m_Scanline[index++] = b0;
+ m_Scanline[index++] = g0;
+ m_Scanline[index++] = r0;
+ }
+ INCR_ON_APPLE(index);
}
-#if BUILDFLAG(IS_APPLE)
- index++;
-#endif
- }
+ });
break;
}
case FXDIB_Format::k1bppMask: {
int m0 = m_RampR[0];
int m1 = m_RampR[255];
int index = 0;
- for (int i = 0; i < m_Width; i++) {
- if (src_buf[i / 8] & (1 << (7 - i % 8)))
- m_Scanline[index++] = m1;
- else
- m_Scanline[index++] = m0;
- }
+ UNSAFE_TODO({
+ for (int i = 0; i < m_Width; i++) {
+ if (src_buf[i / 8] & (1 << (7 - i % 8))) {
+ m_Scanline[index++] = m1;
+ } else {
+ m_Scanline[index++] = m0;
+ }
+ }
+ });
break;
}
case FXDIB_Format::k8bppRgb: {
pdfium::span<const uint32_t> src_palette = m_pSrc->GetPaletteSpan();
int index = 0;
- for (int i = 0; i < m_Width; i++) {
- if (m_pSrc->HasPalette()) {
- FX_ARGB src_argb = src_palette[*src_buf];
- m_Scanline[index++] = m_RampB[FXARGB_R(src_argb)];
- m_Scanline[index++] = m_RampG[FXARGB_G(src_argb)];
- m_Scanline[index++] = m_RampR[FXARGB_B(src_argb)];
- } else {
- uint32_t src_byte = *src_buf;
- m_Scanline[index++] = m_RampB[src_byte];
- m_Scanline[index++] = m_RampG[src_byte];
- m_Scanline[index++] = m_RampR[src_byte];
+ UNSAFE_TODO({
+ for (int i = 0; i < m_Width; i++) {
+ if (m_pSrc->HasPalette()) {
+ FX_ARGB src_argb = src_palette[*src_buf];
+ m_Scanline[index++] = m_RampB[FXARGB_R(src_argb)];
+ m_Scanline[index++] = m_RampG[FXARGB_G(src_argb)];
+ m_Scanline[index++] = m_RampR[FXARGB_B(src_argb)];
+ } else {
+ uint32_t src_byte = *src_buf;
+ m_Scanline[index++] = m_RampB[src_byte];
+ m_Scanline[index++] = m_RampG[src_byte];
+ m_Scanline[index++] = m_RampR[src_byte];
+ }
+ src_buf++;
+ INCR_ON_APPLE(index);
}
- src_buf++;
-#if BUILDFLAG(IS_APPLE)
- index++;
-#endif
- }
+ });
break;
}
case FXDIB_Format::k8bppMask: {
int index = 0;
- for (int i = 0; i < m_Width; i++)
- m_Scanline[index++] = m_RampR[*(src_buf++)];
+ UNSAFE_TODO({
+ for (int i = 0; i < m_Width; i++) {
+ m_Scanline[index++] = m_RampR[*(src_buf++)];
+ }
+ });
break;
}
case FXDIB_Format::kRgb: {
int index = 0;
- for (int i = 0; i < m_Width; i++) {
- m_Scanline[index++] = m_RampB[*(src_buf++)];
- m_Scanline[index++] = m_RampG[*(src_buf++)];
- m_Scanline[index++] = m_RampR[*(src_buf++)];
-#if BUILDFLAG(IS_APPLE)
- index++;
-#endif
- }
+ UNSAFE_TODO({
+ for (int i = 0; i < m_Width; i++) {
+ m_Scanline[index++] = m_RampB[*(src_buf++)];
+ m_Scanline[index++] = m_RampG[*(src_buf++)];
+ m_Scanline[index++] = m_RampR[*(src_buf++)];
+ INCR_ON_APPLE(index);
+ }
+ });
break;
}
case FXDIB_Format::kRgb32:
@@ -133,19 +141,19 @@
[[fallthrough]];
case FXDIB_Format::kArgb: {
int index = 0;
- for (int i = 0; i < m_Width; i++) {
- m_Scanline[index++] = m_RampB[*(src_buf++)];
- m_Scanline[index++] = m_RampG[*(src_buf++)];
- m_Scanline[index++] = m_RampR[*(src_buf++)];
- if (!bSkip) {
- m_Scanline[index++] = *src_buf;
-#if BUILDFLAG(IS_APPLE)
- } else {
- index++;
-#endif
+ UNSAFE_TODO({
+ for (int i = 0; i < m_Width; i++) {
+ m_Scanline[index++] = m_RampB[*(src_buf++)];
+ m_Scanline[index++] = m_RampG[*(src_buf++)];
+ m_Scanline[index++] = m_RampR[*(src_buf++)];
+ if (!bSkip) {
+ m_Scanline[index++] = *src_buf;
+ } else {
+ INCR_ON_APPLE(index);
+ }
+ src_buf++;
}
- src_buf++;
- }
+ });
break;
}
default:
