Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1018)

Issue 1143663004: Add safe FX_Alloc2D() macro (Closed)

Created:
5 years, 7 months ago by Tom Sepez
Modified:
5 years, 7 months ago
Reviewers:
Lei Zhang
CC:
pdfium-reviews_googlegroups.com
Base URL:
https://pdfium.googlesource.com/pdfium.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

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 Committed: https://pdfium.googlesource.com/pdfium/+/31b3a2b31a50f83ed100e01485013fd871399f45

Patch Set 1 #

Total comments: 4

Patch Set 2 : Drop one file, indent. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+47 lines, -56 lines) Patch
M core/include/fxcrt/fx_basic.h View 1 chunk +1 line, -4 lines 0 comments Download
M core/include/fxcrt/fx_memory.h View 2 chunks +9 lines, -0 lines 0 comments Download
M core/src/fpdfapi/fpdf_edit/fpdf_edit_image.cpp View 1 chunk +1 line, -1 line 0 comments Download
M core/src/fpdfapi/fpdf_font/fpdf_font_cid.cpp View 1 chunk +1 line, -1 line 0 comments Download
M core/src/fpdfapi/fpdf_page/fpdf_page_colors.cpp View 3 chunks +4 lines, -4 lines 0 comments Download
M core/src/fpdfapi/fpdf_page/fpdf_page_func.cpp View 4 chunks +5 lines, -5 lines 0 comments Download
M core/src/fpdfapi/fpdf_parser/fpdf_parser_filters.cpp View 1 chunk +1 line, -1 line 0 comments Download
M core/src/fpdfapi/fpdf_render/fpdf_render_cache.cpp View 1 chunk +1 line, -1 line 0 comments Download
M core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp View 1 chunk +1 line, -1 line 0 comments Download
M core/src/fpdftext/fpdf_text.cpp View 1 chunk +3 lines, -4 lines 0 comments Download
M core/src/fxcodec/codec/fx_codec_fax.cpp View 1 chunk +1 line, -4 lines 0 comments Download
M core/src/fxcodec/codec/fx_codec_flate.cpp View 1 3 chunks +3 lines, -7 lines 0 comments Download
M core/src/fxcodec/codec/fx_codec_jpeg.cpp View 1 chunk +1 line, -4 lines 0 comments Download
M core/src/fxcrt/fx_basic_array.cpp View 1 chunk +1 line, -4 lines 0 comments Download
M core/src/fxcrt/fx_basic_memmgr_unittest.cpp View 2 chunks +7 lines, -0 lines 0 comments Download
M core/src/fxge/agg/agg23/fx_agg_path_storage.cpp View 1 1 chunk +1 line, -4 lines 0 comments Download
M core/src/fxge/dib/fx_dib_engine.cpp View 1 chunk +1 line, -4 lines 0 comments Download
M core/src/fxge/skia/fx_skia_device.cpp View 1 chunk +1 line, -1 line 0 comments Download
M core/src/fxge/win32/fx_win32_gdipext.cpp View 1 chunk +4 lines, -6 lines 0 comments Download

Messages

Total messages: 6 (1 generated)
Tom Sepez
Lei, for review. Something I've thought about for a while ...
5 years, 7 months ago (2015-05-15 23:26:30 UTC) #2
Lei Zhang
https://codereview.chromium.org/1143663004/diff/1/core/src/fxcrt/fx_basic_plex.cpp File core/src/fxcrt/fx_basic_plex.cpp (right): https://codereview.chromium.org/1143663004/diff/1/core/src/fxcrt/fx_basic_plex.cpp#newcode11 core/src/fxcrt/fx_basic_plex.cpp:11: CFX_Plex* p = (CFX_Plex*)FX_Alloc(FX_BYTE, sizeof(CFX_Plex) + nMax * cbElement); ...
5 years, 7 months ago (2015-05-15 23:44:17 UTC) #3
Tom Sepez
https://codereview.chromium.org/1143663004/diff/1/core/src/fxcrt/fx_basic_plex.cpp File core/src/fxcrt/fx_basic_plex.cpp (right): https://codereview.chromium.org/1143663004/diff/1/core/src/fxcrt/fx_basic_plex.cpp#newcode11 core/src/fxcrt/fx_basic_plex.cpp:11: CFX_Plex* p = (CFX_Plex*)FX_Alloc(FX_BYTE, sizeof(CFX_Plex) + nMax * cbElement); ...
5 years, 7 months ago (2015-05-18 16:04:31 UTC) #4
Lei Zhang
lgtm
5 years, 7 months ago (2015-05-18 20:27:28 UTC) #5
Tom Sepez
5 years, 7 months ago (2015-05-18 21:18:18 UTC) #6
Message was sent while issue was closed.
Committed patchset #2 (id:20001) manually as
31b3a2b31a50f83ed100e01485013fd871399f45 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698