Add bounds checks to CWeightTable::Calc() and friends.

Add bounds checks to CWeightTable::Calc() and friends.

BUG=624514
Committed: https://pdfium.googlesource.com/pdfium/+/766901f5ec79b3c3ccd1e872f699642d771a89c5

[Lei Zhang, 2016-08-02 02:03:37]

The CQ bit was checked by thestig@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 02:03:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-02 02:14:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-02 02:14:53]

Dry run: This issue passed the CQ dry run.

[Lei Zhang, 2016-08-02 02:17:25]

thestig@chromium.org changed reviewers:
+ tsepez@chromium.org

[Lei Zhang, 2016-08-02 02:19:09]

CWeightTable::Calc() may be doing calculation incorrectly, but when I tried to
fix them so that they make more sense, existing corpus tests start rendering
incorrectly. So I'm just taking the heavy handed approach and not letting
anything go out of bounds.

[Tom Sepez, 2016-08-02 18:14:10]

https://codereview.chromium.org/2204773003/diff/40001/core/fxge/dib/fx_dib_en...
File core/fxge/dib/fx_dib_engine.cpp (right):

https://codereview.chromium.org/2204773003/diff/40001/core/fxge/dib/fx_dib_en...
core/fxge/dib/fx_dib_engine.cpp:240: if (pWeight >= m_pWeightTablesEnd)
We'll want to store the max index rather than the max ptr.  This test is
technically undefined behaviour (since you risk getting burned on ancient
segmented architectures).  We're only allowed to do pointer comparisons with
pointers that point to the same object or one beyond the end of the object per C
standards.

[Lei Zhang, 2016-08-04 06:15:37]

The CQ bit was checked by thestig@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-04 06:15:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-04 06:30:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-04 06:30:41]

Dry run: This issue passed the CQ dry run.

[Lei Zhang, 2016-08-04 06:33:51]

Patchset #4 (id:60001) has been deleted

[Lei Zhang, 2016-08-04 06:34:02]

https://codereview.chromium.org/2204773003/diff/40001/core/fxge/dib/fx_dib_en...
File core/fxge/dib/fx_dib_engine.cpp (right):

https://codereview.chromium.org/2204773003/diff/40001/core/fxge/dib/fx_dib_en...
core/fxge/dib/fx_dib_engine.cpp:240: if (pWeight >= m_pWeightTablesEnd)
On 2016/08/02 18:14:10, Tom Sepez wrote:
> We'll want to store the max index rather than the max ptr.  This test is
> technically undefined behaviour (since you risk getting burned on ancient
> segmented architectures).  We're only allowed to do pointer comparisons with
> pointers that point to the same object or one beyond the end of the object per
C
> standards. 

Done.

[Tom Sepez, 2016-08-04 17:21:59]

lgtm

https://codereview.chromium.org/2204773003/diff/80001/core/fxge/dib/fx_dib_en...
File core/fxge/dib/fx_dib_engine.cpp (right):

https://codereview.chromium.org/2204773003/diff/80001/core/fxge/dib/fx_dib_en...
core/fxge/dib/fx_dib_engine.cpp:258: size_t idx = index - pWeight->m_SrcStart;
Technically, this subtraction hits the same issue, but its too much work to make
it compliant, and works in practice.

[Tom Sepez, 2016-08-04 17:28:08]

https://codereview.chromium.org/2204773003/diff/80001/core/fxge/dib/fx_dib_en...
File core/fxge/dib/fx_dib_engine.cpp (right):

https://codereview.chromium.org/2204773003/diff/80001/core/fxge/dib/fx_dib_en...
core/fxge/dib/fx_dib_engine.cpp:258: size_t idx = index - pWeight->m_SrcStart;
On 2016/08/04 17:21:58, Tom Sepez wrote:
> Technically, this subtraction hits the same issue, but its too much work to
make
> it compliant, and works in practice.

NM, I thought you were subtracting pWeight, I should read closer.

[Lei Zhang, 2016-08-04 17:34:20]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2016-08-04 17:34:27]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-04 17:34:42]

Description was changed from

==========
Add bounds checks to CWeightTable::Calc() and friends.

BUG=624514
==========

to

==========
Add bounds checks to CWeightTable::Calc() and friends.

BUG=624514

Committed:
https://pdfium.googlesource.com/pdfium/+/766901f5ec79b3c3ccd1e872f699642d771a...
==========

[commit-bot: I haz the power, 2016-08-04 17:34:43]

Committed patchset #4 (id:80001) as
https://pdfium.googlesource.com/pdfium/+/766901f5ec79b3c3ccd1e872f699642d771a...