Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(11)

Issue 2202013002: Bound total pixels in JBig2 images to avoid overflows later. (Closed)

Created:
2 years, 9 months ago by Tom Sepez
Modified:
2 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
Project:
pdfium
Visibility:
Public.

Description

Bound total pixels in JBig2 images to avoid overflows later. Also make these private to ensure they aren't modified so as to violate the bounds checks applied at creation time. BUG=633002 Committed: https://pdfium.googlesource.com/pdfium/+/e21501d9427539828b5d547b9d20a752d06914aa

Patch Set 1 #

Total comments: 4

Patch Set 2 : Use maxium values that are safe #

Patch Set 3 : strictly more correct #

Patch Set 4 : Dont realloc external pointers #

Patch Set 5 : Add tests #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+214 lines, -99 lines) Patch
M BUILD.gn View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M core/fxcodec/jbig2/JBig2_Context.cpp View 6 chunks +7 lines, -7 lines 0 comments Download
M core/fxcodec/jbig2/JBig2_GrdProc.cpp View 10 chunks +11 lines, -11 lines 0 comments Download
M core/fxcodec/jbig2/JBig2_GrrdProc.cpp View 3 chunks +10 lines, -10 lines 0 comments Download
M core/fxcodec/jbig2/JBig2_Image.h View 1 2 3 3 chunks +11 lines, -15 lines 0 comments Download
M core/fxcodec/jbig2/JBig2_Image.cpp View 1 2 3 2 chunks +60 lines, -47 lines 0 comments Download
A core/fxcodec/jbig2/JBig2_Image_unittest.cpp View 1 2 3 4 1 chunk +105 lines, -0 lines 1 comment Download
M core/fxcodec/jbig2/JBig2_SddProc.cpp View 1 chunk +1 line, -1 line 0 comments Download
M core/fxcodec/jbig2/JBig2_TrdProc.cpp View 4 chunks +8 lines, -8 lines 0 comments Download

Messages

Total messages: 23 (13 generated)
Tom Sepez
Lei, for review.
2 years, 9 months ago (2016-08-01 22:08:08 UTC) #3
Lei Zhang
lgtm with nits. https://codereview.chromium.org/2202013002/diff/1/core/fxcodec/jbig2/JBig2_Image.cpp File core/fxcodec/jbig2/JBig2_Image.cpp (right): https://codereview.chromium.org/2202013002/diff/1/core/fxcodec/jbig2/JBig2_Image.cpp#newcode18 core/fxcodec/jbig2/JBig2_Image.cpp:18: "Pixel calculatons will overflow"); Are calculatons ...
2 years, 9 months ago (2016-08-01 22:49:50 UTC) #5
Tom Sepez
Lei, re-review required for maximum value change and expand().
2 years, 9 months ago (2016-08-01 23:21:07 UTC) #6
Tom Sepez
https://codereview.chromium.org/2202013002/diff/1/core/fxcodec/jbig2/JBig2_Image.cpp File core/fxcodec/jbig2/JBig2_Image.cpp (right): https://codereview.chromium.org/2202013002/diff/1/core/fxcodec/jbig2/JBig2_Image.cpp#newcode18 core/fxcodec/jbig2/JBig2_Image.cpp:18: "Pixel calculatons will overflow"); On 2016/08/01 22:49:50, Lei Zhang ...
2 years, 9 months ago (2016-08-01 23:21:13 UTC) #7
Lei Zhang
On 2016/08/01 23:21:13, Tom Sepez wrote: > argh. lgtm, but argh indeed. Clearly needs more ...
2 years, 9 months ago (2016-08-01 23:34:28 UTC) #10
Tom Sepez
On 2016/08/01 23:34:28, Lei Zhang wrote: > On 2016/08/01 23:21:13, Tom Sepez wrote: > > ...
2 years, 9 months ago (2016-08-02 19:28:25 UTC) #13
Lei Zhang
lgtm https://codereview.chromium.org/2202013002/diff/80001/core/fxcodec/jbig2/JBig2_Image_unittest.cpp File core/fxcodec/jbig2/JBig2_Image_unittest.cpp (right): https://codereview.chromium.org/2202013002/diff/80001/core/fxcodec/jbig2/JBig2_Image_unittest.cpp#newcode37 core/fxcodec/jbig2/JBig2_Image_unittest.cpp:37: EXPECT_EQ(nullptr, img.m_pData); You can also do EXPECT_FALSE(ptr);
2 years, 9 months ago (2016-08-02 19:36:56 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2202013002/80001
2 years, 9 months ago (2016-08-02 20:36:01 UTC) #20
commit-bot: I haz the power
Committed patchset #5 (id:80001) as https://pdfium.googlesource.com/pdfium/+/e21501d9427539828b5d547b9d20a752d06914aa
2 years, 9 months ago (2016-08-02 20:36:20 UTC) #22
noahman
2 years, 7 months ago (2016-10-24 14:26:27 UTC) #23
Message was sent while issue was closed.
On 2016/08/02 20:36:20, commit-bot: I haz the power wrote:
> Committed patchset #5 (id:80001) as
>
https://pdfium.googlesource.com/pdfium/+/e21501d9427539828b5d547b9d20a752d069...

Here is also possible to track the issue: http://www.buythesisonline.org. Added
back up copy to my project available in the account.

Powered by Google App Engine
This is Rietveld 408576698