diff options
author | Tom Sepez <tsepez@chromium.org> | 2015-05-18 14:18:08 -0700 |
---|---|---|
committer | Tom Sepez <tsepez@chromium.org> | 2015-05-18 14:18:08 -0700 |
commit | 31b3a2b31a50f83ed100e01485013fd871399f45 (patch) | |
tree | aeece5130880a698b56eec044d73925e7e5ae7f3 | |
parent | a88e3a16ae711f6523ad3a40a08d774b72adc9eb (diff) | |
download | pdfium-31b3a2b31a50f83ed100e01485013fd871399f45.tar.xz |
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
19 files changed, 47 insertions, 56 deletions
diff --git a/core/include/fxcrt/fx_basic.h b/core/include/fxcrt/fx_basic.h index bd53d9e7a4..f258029a11 100644 --- a/core/include/fxcrt/fx_basic.h +++ b/core/include/fxcrt/fx_basic.h @@ -1454,10 +1454,7 @@ public: 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 afce91169a..a75ff25818 100644 --- a/core/include/fxcrt/fx_memory.h +++ b/core/include/fxcrt/fx_memory.h @@ -41,6 +41,14 @@ inline void* FX_AllocOrDie(size_t num_members, size_t member_size) { 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 @@ inline void* FX_ReallocOrDie(void* ptr, size_t num_members, size_t member_size) // 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 3a3756de5d..1328fcdf96 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 @@ void CPDF_Image::SetImage(const CFX_DIBitmap* pBitmap, FX_INT32 iCompress, IFX_F 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 6911942c0e..a08660c52f 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 @@ void CPDF_CMapParser::ParseWord(FX_BSTR word) 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 b6bf7950ff..8b9ff8ea96 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 @@ FX_BOOL CPDF_ICCBasedCS::v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) } } 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 @@ void CPDF_ICCBasedCS::TranslateImageLine(FX_LPBYTE pDestBuf, FX_LPCBYTE pSrcBuf, 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 @@ FX_BOOL CPDF_IndexedCS::v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) } 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 bd1cdb6434..8ec490a9e8 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 @@ FX_BOOL CPDF_ExpIntFunc::v_Init(CPDF_Object* pObj) } } 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 @@ FX_BOOL CPDF_StitchFunc::v_Init(CPDF_Object* pObj) 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 @@ FX_BOOL CPDF_Function::Init(CPDF_Object* pObj) 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 @@ FX_BOOL CPDF_Function::Init(CPDF_Object* pObj) 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 ac3f2b29d7..6fa7419a41 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 @@ void CPDF_DecryptFilter::v_FilterFinish(CFX_BinaryBuf& dest_buf) 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 658fc47266..83d5f0a4c1 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 @@ void CPDF_PageRenderCache::CacheOptimization(FX_INT32 dwLimitCacheSize) 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 2d7e1aeb93..f9eec23f7a 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 @@ static void _DrawLatticeGouraudShading(CFX_DIBitmap* pBitmap, CFX_AffineMatrix* 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 6c1e22563d..a0b01042e1 100644 --- a/core/src/fpdftext/fpdf_text.cpp +++ b/core/src/fpdftext/fpdf_text.cpp @@ -57,10 +57,9 @@ void CTextPage::ProcessObject(CPDF_PageObject* pObject) 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 667b713df8..33e89e4f92 100644 --- a/core/src/fxcodec/codec/fx_codec_fax.cpp +++ b/core/src/fxcodec/codec/fx_codec_fax.cpp @@ -949,10 +949,7 @@ CCodec_FaxEncoder::CCodec_FaxEncoder(FX_LPCBYTE src_buf, int width, int height, 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 bbee167f3d..4d43cc554b 100644 --- a/core/src/fxcodec/codec/fx_codec_flate.cpp +++ b/core/src/fxcodec/codec/fx_codec_flate.cpp @@ -13,7 +13,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) { @@ -241,9 +241,7 @@ static FX_BOOL PNG_PredictorEncode(FX_LPBYTE& data_buf, FX_DWORD& data_size, 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 @@ static FX_BOOL PNG_Predictor(FX_LPBYTE& data_buf, FX_DWORD& data_size, 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 d4c9926254..60575d4e34 100644 --- a/core/src/fxcodec/codec/fx_codec_jpeg.cpp +++ b/core/src/fxcodec/codec/fx_codec_jpeg.cpp @@ -153,10 +153,7 @@ static void _JpegEncode(const CFX_DIBSource* pSource, FX_LPBYTE& dest_buf, FX_ST } 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 9bdc607bfe..5a2a2e54a1 100644 --- a/core/src/fxcrt/fx_basic_array.cpp +++ b/core/src/fxcrt/fx_basic_array.cpp @@ -189,10 +189,7 @@ void* CFX_BaseSegmentedArray::Add() 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 565021d29e..c70f3b197d 100644 --- a/core/src/fxcrt/fx_basic_memmgr_unittest.cpp +++ b/core/src/fxcrt/fx_basic_memmgr_unittest.cpp @@ -12,6 +12,8 @@ namespace { 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 @@ TEST(fxcrt, FX_AllocOverflow) { 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 8c4b701ebe..b4b184e0a4 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 @@ void path_storage::allocate_block(unsigned nb) { 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 5053c306f5..7c40171c2b 100644 --- a/core/src/fxge/dib/fx_dib_engine.cpp +++ b/core/src/fxge/dib/fx_dib_engine.cpp @@ -316,10 +316,7 @@ FX_BOOL CStretchEngine::StartStretchHorz() 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 cc4059dd1d..a483eca5ca 100644 --- a/core/src/fxge/skia/fx_skia_device.cpp +++ b/core/src/fxge/skia/fx_skia_device.cpp @@ -210,7 +210,7 @@ static void SkRasterizeStroke(SkPaint& spaint, SkPath* dstPathData, SkPath& path 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 fae1883f4d..49c3f2b0cf 100644 --- a/core/src/fxge/win32/fx_win32_gdipext.cpp +++ b/core/src/fxge/win32/fx_win32_gdipext.cpp @@ -1266,16 +1266,14 @@ CFX_DIBitmap* CGdiplusExt::LoadDIBitmap(WINDIB_Open_Args_ args) 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); |