Commit 682ba0ef authored by Marja Hölttä's avatar Marja Hölttä Committed by V8 LUCI CQ

[valueserializer] Implement a compatibility mode for deserializing broken data

For a while, we shipped a version which writes version 13 data with
JSArrayBufferView flags, and then fixed version 13 to not include the
flags.

This CL adds a compatibility mode for parsing the the version 13
data which includes the flags, since it still occurs in the wild.

Bug: chromium:1314833,chromium:1284506
Change-Id: I96cc432c8574a40b11ec0037394feb1853515760
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3583982Reviewed-by: 's avatarJakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79959}
parent 431da464
...@@ -3496,7 +3496,7 @@ MaybeLocal<Value> ValueDeserializer::ReadValue(Local<Context> context) { ...@@ -3496,7 +3496,7 @@ MaybeLocal<Value> ValueDeserializer::ReadValue(Local<Context> context) {
PREPARE_FOR_EXECUTION(context, ValueDeserializer, ReadValue, Value); PREPARE_FOR_EXECUTION(context, ValueDeserializer, ReadValue, Value);
i::MaybeHandle<i::Object> result; i::MaybeHandle<i::Object> result;
if (GetWireFormatVersion() > 0) { if (GetWireFormatVersion() > 0) {
result = private_->deserializer.ReadObject(); result = private_->deserializer.ReadObjectWrapper();
} else { } else {
result = result =
private_->deserializer.ReadObjectUsingEntireBufferForLegacyFormat(); private_->deserializer.ReadObjectUsingEntireBufferForLegacyFormat();
......
...@@ -1371,6 +1371,32 @@ void ValueDeserializer::TransferArrayBuffer( ...@@ -1371,6 +1371,32 @@ void ValueDeserializer::TransferArrayBuffer(
} }
} }
MaybeHandle<Object> ValueDeserializer::ReadObjectWrapper() {
// We had a bug which produced invalid version 13 data (see
// crbug.com/1284506). This compatibility mode tries to first read the data
// normally, and if it fails, and the version is 13, tries to read the broken
// format.
const uint8_t* original_position = position_;
suppress_deserialization_errors_ = true;
MaybeHandle<Object> result = ReadObject();
// The deserialization code doesn't throw errors for invalid data. It throws
// errors for stack overflows, though, and in that case we won't retry.
if (result.is_null() && version_ == 13 &&
!isolate_->has_pending_exception()) {
version_13_broken_data_mode_ = true;
position_ = original_position;
result = ReadObject();
}
if (result.is_null() && !isolate_->has_pending_exception()) {
isolate_->Throw(*isolate_->factory()->NewError(
MessageTemplate::kDataCloneDeserializationError));
}
return result;
}
MaybeHandle<Object> ValueDeserializer::ReadObject() { MaybeHandle<Object> ValueDeserializer::ReadObject() {
DisallowJavascriptExecution no_js(isolate_); DisallowJavascriptExecution no_js(isolate_);
// If we are at the end of the stack, abort. This function may recurse. // If we are at the end of the stack, abort. This function may recurse.
...@@ -1388,7 +1414,8 @@ MaybeHandle<Object> ValueDeserializer::ReadObject() { ...@@ -1388,7 +1414,8 @@ MaybeHandle<Object> ValueDeserializer::ReadObject() {
result = ReadJSArrayBufferView(Handle<JSArrayBuffer>::cast(object)); result = ReadJSArrayBufferView(Handle<JSArrayBuffer>::cast(object));
} }
if (result.is_null() && !isolate_->has_pending_exception()) { if (result.is_null() && !suppress_deserialization_errors_ &&
!isolate_->has_pending_exception()) {
isolate_->Throw(*isolate_->factory()->NewError( isolate_->Throw(*isolate_->factory()->NewError(
MessageTemplate::kDataCloneDeserializationError)); MessageTemplate::kDataCloneDeserializationError));
} }
...@@ -1964,7 +1991,8 @@ MaybeHandle<JSArrayBufferView> ValueDeserializer::ReadJSArrayBufferView( ...@@ -1964,7 +1991,8 @@ MaybeHandle<JSArrayBufferView> ValueDeserializer::ReadJSArrayBufferView(
byte_length > buffer_byte_length - byte_offset) { byte_length > buffer_byte_length - byte_offset) {
return MaybeHandle<JSArrayBufferView>(); return MaybeHandle<JSArrayBufferView>();
} }
if (version_ >= 14 && !ReadVarint<uint32_t>().To(&flags)) { const bool should_read_flags = version_ >= 14 || version_13_broken_data_mode_;
if (should_read_flags && !ReadVarint<uint32_t>().To(&flags)) {
return MaybeHandle<JSArrayBufferView>(); return MaybeHandle<JSArrayBufferView>();
} }
uint32_t id = next_id_++; uint32_t id = next_id_++;
......
...@@ -212,7 +212,7 @@ class ValueDeserializer { ...@@ -212,7 +212,7 @@ class ValueDeserializer {
/* /*
* Deserializes a V8 object from the buffer. * Deserializes a V8 object from the buffer.
*/ */
MaybeHandle<Object> ReadObject() V8_WARN_UNUSED_RESULT; MaybeHandle<Object> ReadObjectWrapper() V8_WARN_UNUSED_RESULT;
/* /*
* Reads an object, consuming the entire buffer. * Reads an object, consuming the entire buffer.
...@@ -256,6 +256,7 @@ class ValueDeserializer { ...@@ -256,6 +256,7 @@ class ValueDeserializer {
Maybe<double> ReadDouble() V8_WARN_UNUSED_RESULT; Maybe<double> ReadDouble() V8_WARN_UNUSED_RESULT;
Maybe<base::Vector<const uint8_t>> ReadRawBytes(size_t size) Maybe<base::Vector<const uint8_t>> ReadRawBytes(size_t size)
V8_WARN_UNUSED_RESULT; V8_WARN_UNUSED_RESULT;
MaybeHandle<Object> ReadObject() V8_WARN_UNUSED_RESULT;
// Reads a string if it matches the one provided. // Reads a string if it matches the one provided.
// Returns true if this was the case. Otherwise, nothing is consumed. // Returns true if this was the case. Otherwise, nothing is consumed.
...@@ -322,6 +323,8 @@ class ValueDeserializer { ...@@ -322,6 +323,8 @@ class ValueDeserializer {
const bool supports_shared_values_; const bool supports_shared_values_;
uint32_t version_ = 0; uint32_t version_ = 0;
uint32_t next_id_ = 0; uint32_t next_id_ = 0;
bool version_13_broken_data_mode_ = false;
bool suppress_deserialization_errors_ = false;
// Always global handles. // Always global handles.
Handle<FixedArray> id_map_; Handle<FixedArray> id_map_;
......
...@@ -2334,6 +2334,23 @@ TEST_F(ValueSerializerTest, DecodeTypedArrayBackwardsCompatiblity) { ...@@ -2334,6 +2334,23 @@ TEST_F(ValueSerializerTest, DecodeTypedArrayBackwardsCompatiblity) {
}); });
} }
TEST_F(ValueSerializerTest, DecodeTypedArrayBrokenData) {
// Test decoding the broken data where the version is 13 but the
// JSArrayBufferView flags are present.
// The data below is produced by the following code + changing the version
// to 13:
// std::vector<uint8_t> encoded =
// EncodeTest("({ a: new Uint8Array(), b: 13 })");
Local<Value> value = DecodeTest({0xFF, 0xD, 0x6F, 0x22, 0x1, 0x61, 0x42,
0x0, 0x56, 0x42, 0x0, 0x0, 0xE8, 0x47,
0x22, 0x1, 0x62, 0x49, 0x1A, 0x7B, 0x2});
ASSERT_TRUE(value->IsObject());
ExpectScriptTrue("Object.getPrototypeOf(result.a) === Uint8Array.prototype");
ExpectScriptTrue("result.b === 13");
}
TEST_F(ValueSerializerTest, DecodeInvalidTypedArray) { TEST_F(ValueSerializerTest, DecodeInvalidTypedArray) {
// Byte offset out of range. // Byte offset out of range.
InvalidDecodeTest( InvalidDecodeTest(
...@@ -3281,5 +3298,20 @@ TEST_F(ValueSerializerTest, NonStringErrorStack) { ...@@ -3281,5 +3298,20 @@ TEST_F(ValueSerializerTest, NonStringErrorStack) {
EXPECT_TRUE(stack->IsUndefined()); EXPECT_TRUE(stack->IsUndefined());
} }
TEST_F(ValueSerializerTest, InvalidLegacyFormatData) {
std::vector<uint8_t> data = {0xFF, 0x0, 0xDE, 0xAD, 0xDA, 0xDA};
Local<Context> context = deserialization_context();
Context::Scope scope(context);
TryCatch try_catch(isolate());
ValueDeserializer deserializer(isolate(), &data[0],
static_cast<int>(data.size()),
GetDeserializerDelegate());
deserializer.SetSupportsLegacyWireFormat(true);
BeforeDecode(&deserializer);
CHECK(deserializer.ReadHeader(context).FromMaybe(false));
CHECK(deserializer.ReadValue(context).IsEmpty());
CHECK(try_catch.HasCaught());
}
} // namespace } // namespace
} // namespace v8 } // namespace v8
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment