Fix another FastMalloc call site with potential integer overflow.

Fix another FastMalloc call site with potential integer overflow.

BUG=669751
Review-Url: https://codereview.chromium.org/2957933002
Cr-Commit-Position: refs/heads/master@{#482676}
Committed: https://chromium.googlesource.com/chromium/src/+/a62b55155c2812142c3e6d10a51dc5cd33d0fc7f

[palmer, 2017-06-26 22:08:34]

Description was changed from

==========
Fix some more FastMalloc call sites with potential integer overflow.

BUG=669751
==========

to

==========
Fix some another FastMalloc call site with potential integer overflow.

BUG=669751
==========

[palmer, 2017-06-26 22:08:34]

palmer@chromium.org changed reviewers:
+ jochen@chromium.org

[palmer, 2017-06-26 22:08:48]

Here's a small CL for you.

[palmer, 2017-06-26 22:14:06]

Description was changed from

==========
Fix some another FastMalloc call site with potential integer overflow.

BUG=669751
==========

to

==========
Fix another FastMalloc call site with potential integer overflow.

BUG=669751
==========

[palmer, 2017-06-26 22:14:13]

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

[commit-bot: I haz the power, 2017-06-26 22:14:28]

Dry run: CQ is trying da patch.

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

[Tom Sepez, 2017-06-26 22:25:21]

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

[Tom Sepez, 2017-06-26 22:25:21]

I'm trying to convince myself whether any of these overflows can actually
happen, since the size is coming from a pre-existing object, and there has to be
memory to hold it, subject to all the total space limitations we put on the
partitions.

I suppose if you were going from an array of char to an equaly-sized array of
large_struct, there might be an issue, but were going from a map entry to two
pointers ...

[Tom Sepez, 2017-06-26 22:33:37]

https://codereview.chromium.org/2957933002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/xml/XSLTProcessorLibxslt.cpp (right):

https://codereview.chromium.org/2957933002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/xml/XSLTProcessorLibxslt.cpp:234:
WTF::CheckedSizeT size = parameters.size();
Blink is pretty slap-happy about using "unsigned" for counts, lengths, etc, so
if the caclulation overflows a uint32 we probably want to bail, even if we have
64-bit size_ts.

[commit-bot: I haz the power, 2017-06-27 00:17:57]

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

[commit-bot: I haz the power, 2017-06-27 00:17:58]

Dry run: This issue passed the CQ dry run.

[palmer, 2017-06-27 01:07:50]

> I'm trying to convince myself whether any of these overflows can actually
happen, since the size is coming from a pre-existing object, and there has to be
memory to hold it, subject to all the total space limitations we put on the
partitions.

Right. But:

* I'd like it to be obviously safe, without requiring us to have to convince
ourselves, and spend time convincing ourselves. :)

* If the allocation size overflows to a small value, it might not fall afoul of
partition size limits, yet still result in a vulnerable situation (e.g.
subsequent code assuming mor eobjects/a larger total size).

[palmer, 2017-06-27 01:08:57]

> Blink is pretty slap-happy about using "unsigned" for counts, lengths, etc, so
if the caclulation overflows a uint32 we probably want to bail, even if we have
64-bit size_ts.

Yeah, I was thinking about that. What do you think of enforcing that with a
checked_cast<unsigned>?

[jochen (gone - plz use gerrit), 2017-06-27 09:29:57]

lgtm

[palmer, 2017-06-27 17:48:06]

The CQ bit was checked by palmer@chromium.org

[commit-bot: I haz the power, 2017-06-27 17:48:20]

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, 2017-06-27 17:51:44]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1498585686948400, "parent_rev":
"2795024de687133204d9642e379d312431cae712", "commit_rev":
"a62b55155c2812142c3e6d10a51dc5cd33d0fc7f"}

[commit-bot: I haz the power, 2017-06-27 17:52:01]

Description was changed from

==========
Fix another FastMalloc call site with potential integer overflow.

BUG=669751
==========

to

==========
Fix another FastMalloc call site with potential integer overflow.

BUG=669751

Review-Url: https://codereview.chromium.org/2957933002
Cr-Commit-Position: refs/heads/master@{#482676}
Committed:
https://chromium.googlesource.com/chromium/src/+/a62b55155c2812142c3e6d10a51d...
==========

[commit-bot: I haz the power, 2017-06-27 17:52:06]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/a62b55155c2812142c3e6d10a51d...