AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

Fix two overflow cases in SourceMap VLQ decoding

Browse Source 
These both have to do with extremely large numbers, so it's unlikely to cause a problem in practice. Still, correctness.

First, encoding `-2147483648` in VLQ returns the value `"B"`. When decoding, we get the value `1` after reading the base64. We then check if the first bit is set (it is) to see if we should negate it, then we shift all bits right once. Now, `value` will be `0` and `negate` will be `true`. So, we'd return `-0`. Which is a bug! `-0` isn't `-2147483648`, and we've broken a round trip.

Second, encoding any number with the 31st bit set, we'd return the opposite sign. Let's use `1073741824`. Encoding, we get `"ggggggC"`. When decoding, we get the value `-2147483648` after reading the base64. Notice, it's already negative (the 32nd bit is set, because the 31st was set and we shifted everything left once). We'd then check the first bit (it's not) and shift right. But we used `>>`, which does not shift the sign bit. We actually wanted `>>>`, which will. Because of that bug, we get back `-1073741824` instead of the positive `1073741824`. It's even worse if the 32nd and 31st bits are set, `-1610612736` becomes `536870912` after a round trip.

I recently fixed the same two bugs in Closure Compiler: https://github.com/google/closure-compiler/commit/584418eb

Change-Id: Ib6592ad50ae3764479c1a766bbb19042ee83b99d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2018882
Auto-Submit: Justin Ridgewell <jridgewell@google.com>
Commit-Queue: Mathias Bynens <mathias@chromium.org>
Reviewed-by: Mathias Bynens <mathias@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65987}
This commit is contained in:
Justin Ridgewell 2020-01-24 02:39:59 -05:00 committed by Commit Bot
parent 7d1f95d6e4
commit 615ecdf8c9
1 changed files with 13 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
tools/SourceMap.js
View File

@ -325,8 +325,19 @@ WebInspector.SourceMap.prototype = {
// Fix the sign.
var negative = result & 1;
result >>= 1;
return negative ? -result : result;
// Use unsigned right shift, so that the 32nd bit is properly shifted
// to the 31st, and the 32nd becomes unset.
result >>>= 1;
if (negate) {
// We need to OR 0x80000000 here to ensure the 32nd bit (the sign bit
// in a 32bit int) is always set for negative numbers. If `result`
// were 1, (meaning `negate` is true and all other bits were zeros),
// `result` would now be 0. But -0 doesn't flip the 32nd bit as
// intended. All other numbers will successfully set the 32nd bit
// without issue, so doing this is a noop for them.
return -result | 0x80000000;
}
return result;
},
_VLQ_BASE_SHIFT: 5,