Add safe FX_Alloc2D() macro
This avoids unchecked multiplications when computing a size argument
to malloc(). Such an overflow is very scary, and can result in
exploitable bugs.
Along the way, kill off some return checks, since we know this can't
return NULL.
R=thestig@chromium.org
Review URL: https://codereview.chromium.org/1143663004
diff --git a/core/include/fxcrt/fx_basic.h b/core/include/fxcrt/fx_basic.h
index bd53d9e..f258029 100644
--- a/core/include/fxcrt/fx_basic.h
+++ b/core/include/fxcrt/fx_basic.h
@@ -1454,10 +1454,7 @@
while (nCount > 0) {
FX_INT32 temp_count = FX_MIN(nCount, FX_DATALIST_LENGTH);
DataList list;
- list.data = FX_Alloc(FX_BYTE, temp_count * unit);
- if (!list.data) {
- break;
- }
+ list.data = FX_Alloc2D(FX_BYTE, temp_count, unit);
list.start = nStart;
list.count = temp_count;
Append(list);
diff --git a/core/include/fxcrt/fx_memory.h b/core/include/fxcrt/fx_memory.h
index afce911..a75ff25 100644
--- a/core/include/fxcrt/fx_memory.h
+++ b/core/include/fxcrt/fx_memory.h
@@ -41,6 +41,14 @@
return nullptr; // Suppress compiler warning.
}
+inline void* FX_AllocOrDie2D(size_t w, size_t h, size_t member_size) {
+ if (w < std::numeric_limits<size_t>::max() / h) {
+ return FX_AllocOrDie(w * h, member_size);
+ }
+ FX_OutOfMemoryTerminate(); // Never returns.
+ return nullptr; // Suppress compiler warning.
+}
+
inline void* FX_ReallocOrDie(void* ptr, size_t num_members, size_t member_size) {
if (void* result = FX_SafeRealloc(ptr, num_members, member_size)) {
return result;
@@ -51,6 +59,7 @@
// Never returns NULL.
#define FX_Alloc(type, size) (type*)FX_AllocOrDie(size, sizeof(type))
+#define FX_Alloc2D(type, w, h) (type*)FX_AllocOrDie2D(w, h, sizeof(type))
#define FX_Realloc(type, ptr, size) \
(type*)FX_ReallocOrDie(ptr, size, sizeof(type))
diff --git a/core/src/fpdfapi/fpdf_edit/fpdf_edit_image.cpp b/core/src/fpdfapi/fpdf_edit/fpdf_edit_image.cpp
index 3a3756d..1328fcd 100644
--- a/core/src/fpdfapi/fpdf_edit/fpdf_edit_image.cpp
+++ b/core/src/fpdfapi/fpdf_edit/fpdf_edit_image.cpp
@@ -162,7 +162,7 @@
pCS->AddName(FX_BSTRC("Indexed"));
pCS->AddName(FX_BSTRC("DeviceRGB"));
pCS->AddInteger(iPalette - 1);
- FX_LPBYTE pColorTable = FX_Alloc(FX_BYTE, iPalette * 3);
+ FX_LPBYTE pColorTable = FX_Alloc2D(FX_BYTE, iPalette, 3);
FX_LPBYTE ptr = pColorTable;
for (FX_INT32 i = 0; i < iPalette; i ++) {
FX_DWORD argb = pBitmap->GetPaletteArgb(i);
diff --git a/core/src/fpdfapi/fpdf_font/fpdf_font_cid.cpp b/core/src/fpdfapi/fpdf_font/fpdf_font_cid.cpp
index 6911942..a08660c 100644
--- a/core/src/fpdfapi/fpdf_font/fpdf_font_cid.cpp
+++ b/core/src/fpdfapi/fpdf_font/fpdf_font_cid.cpp
@@ -254,7 +254,7 @@
if (nSegs > 1) {
m_pCMap->m_CodingScheme = CPDF_CMap::MixedFourBytes;
m_pCMap->m_nCodeRanges = nSegs;
- m_pCMap->m_pLeadingBytes = FX_Alloc(FX_BYTE, nSegs * sizeof(_CMap_CodeRange));
+ m_pCMap->m_pLeadingBytes = FX_Alloc2D(FX_BYTE, nSegs, sizeof(_CMap_CodeRange));
FXSYS_memcpy32(m_pCMap->m_pLeadingBytes, m_CodeRanges.GetData(), nSegs * sizeof(_CMap_CodeRange));
} else if (nSegs == 1) {
m_pCMap->m_CodingScheme = (m_CodeRanges[0].m_CharSize == 2) ? CPDF_CMap::TwoBytes : CPDF_CMap::OneByte;
diff --git a/core/src/fpdfapi/fpdf_page/fpdf_page_colors.cpp b/core/src/fpdfapi/fpdf_page/fpdf_page_colors.cpp
index b6bf795..8b9ff8e 100644
--- a/core/src/fpdfapi/fpdf_page/fpdf_page_colors.cpp
+++ b/core/src/fpdfapi/fpdf_page/fpdf_page_colors.cpp
@@ -643,7 +643,7 @@
}
}
CPDF_Array* pRanges = pDict->GetArray(FX_BSTRC("Range"));
- m_pRanges = FX_Alloc(FX_FLOAT, m_nComponents * 2);
+ m_pRanges = FX_Alloc2D(FX_FLOAT, m_nComponents, 2);
for (int i = 0; i < m_nComponents * 2; i ++) {
if (pRanges) {
m_pRanges[i] = pRanges->GetNumber(i);
@@ -715,8 +715,8 @@
CPDF_ModuleMgr::Get()->GetIccModule()->TranslateScanline(m_pProfile->m_pTransform, pDestBuf, pSrcBuf, pixels);
} else {
if (m_pCache == NULL) {
- ((CPDF_ICCBasedCS*)this)->m_pCache = FX_Alloc(FX_BYTE, nMaxColors * 3);
- FX_LPBYTE temp_src = FX_Alloc(FX_BYTE, nMaxColors * m_nComponents);
+ ((CPDF_ICCBasedCS*)this)->m_pCache = FX_Alloc2D(FX_BYTE, nMaxColors, 3);
+ FX_LPBYTE temp_src = FX_Alloc2D(FX_BYTE, nMaxColors, m_nComponents);
FX_LPBYTE pSrc = temp_src;
for (int i = 0; i < nMaxColors; i ++) {
FX_DWORD color = i;
@@ -804,7 +804,7 @@
}
m_pCountedBaseCS = pDocPageData->FindColorSpacePtr(m_pBaseCS->GetArray());
m_nBaseComponents = m_pBaseCS->CountComponents();
- m_pCompMinMax = FX_Alloc(FX_FLOAT, m_nBaseComponents * 2);
+ m_pCompMinMax = FX_Alloc2D(FX_FLOAT, m_nBaseComponents, 2);
FX_FLOAT defvalue;
for (int i = 0; i < m_nBaseComponents; i ++) {
m_pBaseCS->GetDefaultValue(i, defvalue, m_pCompMinMax[i * 2], m_pCompMinMax[i * 2 + 1]);
diff --git a/core/src/fpdfapi/fpdf_page/fpdf_page_func.cpp b/core/src/fpdfapi/fpdf_page/fpdf_page_func.cpp
index bd1cdb6..8ec490a 100644
--- a/core/src/fpdfapi/fpdf_page/fpdf_page_func.cpp
+++ b/core/src/fpdfapi/fpdf_page/fpdf_page_func.cpp
@@ -670,8 +670,8 @@
}
}
CPDF_Array* pArray1 = pDict->GetArray(FX_BSTRC("C1"));
- m_pBeginValues = FX_Alloc(FX_FLOAT, m_nOutputs * 2);
- m_pEndValues = FX_Alloc(FX_FLOAT, m_nOutputs * 2);
+ m_pBeginValues = FX_Alloc2D(FX_FLOAT, m_nOutputs, 2);
+ m_pEndValues = FX_Alloc2D(FX_FLOAT, m_nOutputs, 2);
for (int i = 0; i < m_nOutputs; i ++) {
m_pBeginValues[i] = pArray0 ? pArray0->GetFloat(i) : 0.0f;
m_pEndValues[i] = pArray1 ? pArray1->GetFloat(i) : 1.0f;
@@ -768,7 +768,7 @@
m_pBounds[i + 1] = pArray->GetFloat(i);
}
m_pBounds[m_nSubs] = m_pDomains[1];
- m_pEncode = FX_Alloc(FX_FLOAT, m_nSubs * 2);
+ m_pEncode = FX_Alloc2D(FX_FLOAT, m_nSubs, 2);
pArray = pDict->GetArray(FX_BSTRC("Encode"));
if (pArray == NULL) {
return FALSE;
@@ -857,7 +857,7 @@
if (m_nInputs == 0) {
return FALSE;
}
- m_pDomains = FX_Alloc(FX_FLOAT, m_nInputs * 2);
+ m_pDomains = FX_Alloc2D(FX_FLOAT, m_nInputs, 2);
for (int i = 0; i < m_nInputs * 2; i ++) {
m_pDomains[i] = pDomains->GetFloat(i);
}
@@ -865,7 +865,7 @@
m_nOutputs = 0;
if (pRanges) {
m_nOutputs = pRanges->GetCount() / 2;
- m_pRanges = FX_Alloc(FX_FLOAT, m_nOutputs * 2);
+ m_pRanges = FX_Alloc2D(FX_FLOAT, m_nOutputs, 2);
for (int i = 0; i < m_nOutputs * 2; i ++) {
m_pRanges[i] = pRanges->GetFloat(i);
}
diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_filters.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_filters.cpp
index ac3f2b2..6fa7419 100644
--- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_filters.cpp
+++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_filters.cpp
@@ -296,7 +296,7 @@
extern "C" {
static void* my_alloc_func (void* opaque, unsigned int items, unsigned int size)
{
- return FX_Alloc(FX_BYTE, items * size);
+ return FX_Alloc2D(FX_BYTE, items, size);
}
static void my_free_func (void* opaque, void* address)
{
diff --git a/core/src/fpdfapi/fpdf_render/fpdf_render_cache.cpp b/core/src/fpdfapi/fpdf_render/fpdf_render_cache.cpp
index 658fc47..83d5f0a 100644
--- a/core/src/fpdfapi/fpdf_render/fpdf_render_cache.cpp
+++ b/core/src/fpdfapi/fpdf_render/fpdf_render_cache.cpp
@@ -43,7 +43,7 @@
return;
}
int nCount = m_ImageCaches.GetCount();
- CACHEINFO* pCACHEINFO = (CACHEINFO*)FX_Alloc(FX_BYTE, (sizeof (CACHEINFO)) * nCount);
+ CACHEINFO* pCACHEINFO = (CACHEINFO*)FX_Alloc2D(FX_BYTE, sizeof(CACHEINFO), nCount);
FX_POSITION pos = m_ImageCaches.GetStartPosition();
int i = 0;
while (pos) {
diff --git a/core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp b/core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp
index 2d7e1ae..f9eec23 100644
--- a/core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp
+++ b/core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp
@@ -422,7 +422,7 @@
if (!stream.Load(pShadingStream, pFuncs, nFuncs, pCS)) {
return;
}
- CPDF_MeshVertex* vertex = FX_Alloc(CPDF_MeshVertex, row_verts * 2);
+ CPDF_MeshVertex* vertex = FX_Alloc2D(CPDF_MeshVertex, row_verts, 2);
if (!stream.GetVertexRow(vertex, row_verts, pObject2Bitmap)) {
FX_Free(vertex);
return;
diff --git a/core/src/fpdftext/fpdf_text.cpp b/core/src/fpdftext/fpdf_text.cpp
index 6c1e225..a0b0104 100644
--- a/core/src/fpdftext/fpdf_text.cpp
+++ b/core/src/fpdftext/fpdf_text.cpp
@@ -57,10 +57,9 @@
CPDF_TextObject* pText = (CPDF_TextObject*)pObject;
CPDF_Font* pFont = pText->m_TextState.GetFont();
int count = pText->CountItems();
- FX_FLOAT* pPosArray = FX_Alloc(FX_FLOAT, count * 2);
- if (pPosArray) {
- pText->CalcCharPos(pPosArray);
- }
+ FX_FLOAT* pPosArray = FX_Alloc2D(FX_FLOAT, count, 2);
+ pText->CalcCharPos(pPosArray);
+
FX_FLOAT fontsize_h = pText->m_TextState.GetFontSizeH();
FX_FLOAT fontsize_v = pText->m_TextState.GetFontSizeV();
FX_DWORD space_charcode = pFont->CharCodeFromUnicode(' ');
diff --git a/core/src/fxcodec/codec/fx_codec_fax.cpp b/core/src/fxcodec/codec/fx_codec_fax.cpp
index 667b713..33e89e4 100644
--- a/core/src/fxcodec/codec/fx_codec_fax.cpp
+++ b/core/src/fxcodec/codec/fx_codec_fax.cpp
@@ -949,10 +949,7 @@
return;
}
FXSYS_memset8(m_pRefLine, 0xff, m_Pitch);
- m_pLineBuf = FX_Alloc(FX_BYTE, m_Pitch * 8);
- if (m_pLineBuf == NULL) {
- return;
- }
+ m_pLineBuf = FX_Alloc2D(FX_BYTE, m_Pitch, 8);
m_DestBuf.EstimateSize(0, 10240);
}
CCodec_FaxEncoder::~CCodec_FaxEncoder()
diff --git a/core/src/fxcodec/codec/fx_codec_flate.cpp b/core/src/fxcodec/codec/fx_codec_flate.cpp
index bbee167..4d43cc5 100644
--- a/core/src/fxcodec/codec/fx_codec_flate.cpp
+++ b/core/src/fxcodec/codec/fx_codec_flate.cpp
@@ -13,7 +13,7 @@
{
static void* my_alloc_func (void* opaque, unsigned int items, unsigned int size)
{
- return FX_Alloc(FX_BYTE, items * size);
+ return FX_Alloc2D(FX_BYTE, items, size);
}
static void my_free_func (void* opaque, void* address)
{
@@ -241,9 +241,7 @@
return FALSE;
const int row_count = (data_size + row_size - 1) / row_size;
const int last_row_size = data_size % row_size;
- FX_LPBYTE dest_buf = FX_Alloc( FX_BYTE, (row_size + 1) * row_count);
- if (dest_buf == NULL)
- return FALSE;
+ FX_LPBYTE dest_buf = FX_Alloc2D(FX_BYTE, row_size + 1, row_count);
int byte_cnt = 0;
FX_LPBYTE pSrcData = data_buf;
FX_LPBYTE pDestData = dest_buf;
@@ -397,9 +395,7 @@
return FALSE;
const int row_count = (data_size + row_size) / (row_size + 1);
const int last_row_size = data_size % (row_size + 1);
- FX_LPBYTE dest_buf = FX_Alloc( FX_BYTE, row_size * row_count);
- if (dest_buf == NULL)
- return FALSE;
+ FX_LPBYTE dest_buf = FX_Alloc2D(FX_BYTE, row_size, row_count);
int byte_cnt = 0;
FX_LPBYTE pSrcData = data_buf;
FX_LPBYTE pDestData = dest_buf;
diff --git a/core/src/fxcodec/codec/fx_codec_jpeg.cpp b/core/src/fxcodec/codec/fx_codec_jpeg.cpp
index d4c9926..60575d4 100644
--- a/core/src/fxcodec/codec/fx_codec_jpeg.cpp
+++ b/core/src/fxcodec/codec/fx_codec_jpeg.cpp
@@ -153,10 +153,7 @@
}
FX_LPBYTE line_buf = NULL;
if (nComponents > 1) {
- line_buf = FX_Alloc(FX_BYTE, width * nComponents);
- if (line_buf == NULL) {
- return;
- }
+ line_buf = FX_Alloc2D(FX_BYTE, width, nComponents);
}
jpeg_set_defaults(&cinfo);
if(quality != 75) {
diff --git a/core/src/fxcrt/fx_basic_array.cpp b/core/src/fxcrt/fx_basic_array.cpp
index 9bdc607..5a2a2e5 100644
--- a/core/src/fxcrt/fx_basic_array.cpp
+++ b/core/src/fxcrt/fx_basic_array.cpp
@@ -189,10 +189,7 @@
if (m_DataSize % m_SegmentSize) {
return GetAt(m_DataSize ++);
}
- void* pSegment = FX_Alloc(FX_BYTE, m_UnitSize * m_SegmentSize);
- if (!pSegment) {
- return NULL;
- }
+ void* pSegment = FX_Alloc2D(FX_BYTE, m_UnitSize, m_SegmentSize);
if (m_pIndex == NULL) {
m_pIndex = pSegment;
m_DataSize ++;
diff --git a/core/src/fxcrt/fx_basic_memmgr_unittest.cpp b/core/src/fxcrt/fx_basic_memmgr_unittest.cpp
index 565021d..c70f3b1 100644
--- a/core/src/fxcrt/fx_basic_memmgr_unittest.cpp
+++ b/core/src/fxcrt/fx_basic_memmgr_unittest.cpp
@@ -12,6 +12,8 @@
const size_t kMaxByteAlloc = std::numeric_limits<size_t>::max();
const size_t kMaxIntAlloc = kMaxByteAlloc / sizeof(int);
const size_t kOverflowIntAlloc = kMaxIntAlloc + 100;
+const size_t kWidth = 640;
+const size_t kOverflowIntAlloc2D = kMaxIntAlloc / kWidth + 10;
} // namespace
@@ -35,6 +37,11 @@
FX_Free(ptr);
}
+TEST(fxcrt, FX_AllocOverflow2D) {
+ EXPECT_DEATH_IF_SUPPORTED(
+ FX_Alloc2D(int, kWidth, kOverflowIntAlloc2D), "");
+}
+
TEST(fxcrt, DISABLED_FX_TryAllocOOM) {
EXPECT_FALSE(FX_TryAlloc(int, kMaxIntAlloc));
diff --git a/core/src/fxge/agg/agg23/fx_agg_path_storage.cpp b/core/src/fxge/agg/agg23/fx_agg_path_storage.cpp
index 8c4b701..b4b184e 100644
--- a/core/src/fxge/agg/agg23/fx_agg_path_storage.cpp
+++ b/core/src/fxge/agg/agg23/fx_agg_path_storage.cpp
@@ -51,10 +51,7 @@
{
if(nb >= m_max_blocks) {
FX_FLOAT** new_coords =
- FX_Alloc( FX_FLOAT*, (m_max_blocks + block_pool) * 2);
- if (!new_coords) {
- return;
- }
+ FX_Alloc2D(FX_FLOAT*, m_max_blocks + block_pool, 2);
unsigned char** new_cmds =
(unsigned char**)(new_coords + m_max_blocks + block_pool);
if(m_coord_blocks) {
diff --git a/core/src/fxge/dib/fx_dib_engine.cpp b/core/src/fxge/dib/fx_dib_engine.cpp
index 5053c30..7c40171 100644
--- a/core/src/fxge/dib/fx_dib_engine.cpp
+++ b/core/src/fxge/dib/fx_dib_engine.cpp
@@ -316,10 +316,7 @@
return FALSE;
}
if (m_pSource && m_bHasAlpha && m_pSource->m_pAlphaMask) {
- m_pExtraAlphaBuf = FX_Alloc(unsigned char, m_SrcClip.Height() * m_ExtraMaskPitch);
- if (!m_pExtraAlphaBuf) {
- return FALSE;
- }
+ m_pExtraAlphaBuf = FX_Alloc2D(unsigned char, m_SrcClip.Height(), m_ExtraMaskPitch);
FX_DWORD size = (m_DestClip.Width() * 8 + 31) / 32 * 4;
m_pDestMaskScanline = FX_TryAlloc(unsigned char, size);
if (!m_pDestMaskScanline) {
diff --git a/core/src/fxge/skia/fx_skia_device.cpp b/core/src/fxge/skia/fx_skia_device.cpp
index cc4059d..a483eca 100644
--- a/core/src/fxge/skia/fx_skia_device.cpp
+++ b/core/src/fxge/skia/fx_skia_device.cpp
@@ -210,7 +210,7 @@
dstPathData->transform(smatrix);
} else {
int count = (pGraphState->m_DashCount+1)/2;
- SkScalar* intervals = FX_Alloc(SkScalar, count* sizeof (SkScalar));
+ SkScalar* intervals = FX_Alloc2D(SkScalar, count, sizeof(SkScalar));
// Set dash pattern
for (int i = 0; i < count; i ++) {
FX_FIXFLOAT on = pGraphState->m_DashArray[i*2];
diff --git a/core/src/fxge/win32/fx_win32_gdipext.cpp b/core/src/fxge/win32/fx_win32_gdipext.cpp
index fae1883..49c3f2b 100644
--- a/core/src/fxge/win32/fx_win32_gdipext.cpp
+++ b/core/src/fxge/win32/fx_win32_gdipext.cpp
@@ -1266,16 +1266,14 @@
int height = abs(pInfo->pbmi->bmiHeader.biHeight);
int width = pInfo->pbmi->bmiHeader.biWidth;
int dest_pitch = (width * pInfo->pbmi->bmiHeader.biBitCount + 31) / 32 * 4;
- LPBYTE pData = FX_Alloc(BYTE, dest_pitch * height);
- if (pData == NULL) {
- FreeDIBitmap(pInfo);
- return NULL;
- }
+ LPBYTE pData = FX_Alloc2D(BYTE, dest_pitch, height);
if (dest_pitch == pInfo->Stride) {
FXSYS_memcpy32(pData, pInfo->pScan0, dest_pitch * height);
- } else for (int i = 0; i < height; i ++) {
+ } else {
+ for (int i = 0; i < height; i ++) {
FXSYS_memcpy32(pData + dest_pitch * i, pInfo->pScan0 + pInfo->Stride * i, dest_pitch);
}
+ }
CFX_DIBitmap* pDIBitmap = _FX_WindowsDIB_LoadFromBuf(pInfo->pbmi, pData, pInfo->pbmi->bmiHeader.biBitCount == 32);
FX_Free(pData);
FreeDIBitmap(pInfo);
