[valuserializer] Add a hard fail mode
Invariant in the normal mode: - If the data is invalid, we'll fail gracefully (no crash, no DCHECK failures) Invariant in the hard fail mode: - If the data is invalid (in a way we can detect), a CHECK fails at the earliest location where we detect the inconsistency Bug: chromium:1381404 Change-Id: Icae077a5c76329018fdb759122297134ae70b897 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4013142 Reviewed-by: Camillo Bruni <cbruni@chromium.org> Commit-Queue: Marja Hölttä <marja@chromium.org> Cr-Commit-Position: refs/heads/main@{#84265}
This commit is contained in:
parent
7c6f2cba36
commit
cf3f222543
6
BUILD.gn
6
BUILD.gn
@ -376,6 +376,9 @@ declare_args() {
|
||||
# Compile V8 using zlib as dependency.
|
||||
# Sets -DV8_USE_ZLIB
|
||||
v8_use_zlib = true
|
||||
|
||||
# Make ValueDeserializer crash if the data to deserialize is invalid.
|
||||
v8_value_deserializer_hard_fail = false
|
||||
}
|
||||
|
||||
# Derived defaults.
|
||||
@ -1055,6 +1058,9 @@ config("features") {
|
||||
if (v8_use_zlib) {
|
||||
defines += [ "V8_USE_ZLIB" ]
|
||||
}
|
||||
if (v8_value_deserializer_hard_fail) {
|
||||
defines += [ "V8_VALUE_DESERIALIZER_HARD_FAIL" ]
|
||||
}
|
||||
}
|
||||
|
||||
config("toolchain") {
|
||||
|
@ -1277,9 +1277,8 @@ Maybe<T> ValueDeserializer::ReadVarint() {
|
||||
// same end state and result.
|
||||
auto previous_position = position_;
|
||||
Maybe<T> maybe_expected_value = ReadVarintLoop<T>();
|
||||
if (v8_flags.fuzzing && maybe_expected_value.IsNothing()) {
|
||||
return maybe_expected_value;
|
||||
}
|
||||
// ReadVarintLoop can't return Nothing here; all such conditions have been
|
||||
// checked above.
|
||||
T expected_value = maybe_expected_value.ToChecked();
|
||||
auto expected_position = position_;
|
||||
position_ = previous_position;
|
||||
@ -1331,12 +1330,11 @@ Maybe<T> ValueDeserializer::ReadVarintLoop() {
|
||||
value |= static_cast<T>(byte & 0x7F) << shift;
|
||||
shift += 7;
|
||||
} else {
|
||||
// We allow arbitrary data to be deserialized when fuzzing.
|
||||
// Since {value} is not modified in this branch we can safely skip the
|
||||
// DCHECK when fuzzing.
|
||||
DCHECK_IMPLIES(!v8_flags.fuzzing, !has_another_byte);
|
||||
// For consistency with the fast unrolled loop in ReadVarint we return
|
||||
// after we have read size(T) + 1 bytes.
|
||||
#ifdef V8_VALUE_DESERIALIZER_HARD_FAIL
|
||||
CHECK(!has_another_byte);
|
||||
#endif // V8_VALUE_DESERIALIZER_HARD_FAIL
|
||||
return Just(value);
|
||||
}
|
||||
position_++;
|
||||
@ -1666,9 +1664,10 @@ bool ValueDeserializer::ReadExpectedString(Handle<String> expected) {
|
||||
return {};
|
||||
}
|
||||
// Length is also checked in ReadRawBytes.
|
||||
DCHECK_IMPLIES(!v8_flags.fuzzing,
|
||||
byte_length <= static_cast<uint32_t>(
|
||||
std::numeric_limits<int32_t>::max()));
|
||||
#ifdef V8_VALUE_DESERIALIZER_HARD_FAIL
|
||||
CHECK_LE(byte_length,
|
||||
static_cast<uint32_t>(std::numeric_limits<int32_t>::max()));
|
||||
#endif // V8_VALUE_DESERIALIZER_HARD_FAIL
|
||||
if (!ReadRawBytes(byte_length).To(&bytes)) {
|
||||
position_ = original_position;
|
||||
return false;
|
||||
|
9
test/mjsunit/regress/regress-crbug-1381404.js
Normal file
9
test/mjsunit/regress/regress-crbug-1381404.js
Normal file
@ -0,0 +1,9 @@
|
||||
// Copyright 2022 the V8 project authors. All rights reserved.
|
||||
// Use of this source code is governed by a BSD-style license that can be
|
||||
// found in the LICENSE file.
|
||||
|
||||
const a = {"maxByteLength": 15061061};
|
||||
const e = d8.serializer.serialize(a);
|
||||
const f = new Uint8Array(e);
|
||||
f[18] = 114;
|
||||
assertThrows(() => { d8.serializer.deserialize(e); });
|
Loading…
Reference in New Issue
Block a user