/
valueserializer_report_if_buffer_expansion_fails_during.patch
164 lines (155 loc) · 5.85 KB
/
valueserializer_report_if_buffer_expansion_fails_during.patch
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Jeremy Roman <jbroman@chromium.org>
Date: Thu, 13 Dec 2018 18:43:00 -0500
Subject: ValueSerializer: Report if buffer expansion fails during
WriteHostObject.
Also fail early if we detect that we've previously run out of memory and thus
corrupted the buffer.
Add a unit test for this kind of case.
Bug: chromium:914731
Change-Id: Iaaf3927209bffeab6fe8ba462d9dd9dad8cbbe2f
Reviewed-on: https://chromium-review.googlesource.com/c/1377449
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58248}
diff --git a/src/value-serializer.cc b/src/value-serializer.cc
index 4c9c9a9aa2d9ce6017bb02448fa474478c9f6308..8c85bd9f5466612937a71f8bfdcb82120f7fb9c7 100644
--- a/src/value-serializer.cc
+++ b/src/value-serializer.cc
@@ -344,7 +344,11 @@ void ValueSerializer::TransferArrayBuffer(uint32_t transfer_id,
}
Maybe<bool> ValueSerializer::WriteObject(Handle<Object> object) {
- out_of_memory_ = false;
+ // There is no sense in trying to proceed if we've previously run out of
+ // memory. Bail immediately, as this likely implies that some write has
+ // previously failed and so the buffer is corrupt.
+ if (V8_UNLIKELY(out_of_memory_)) return ThrowIfOutOfMemory();
+
if (object->IsSmi()) {
WriteSmi(Smi::cast(*object));
return ThrowIfOutOfMemory();
@@ -930,8 +934,10 @@ Maybe<bool> ValueSerializer::WriteHostObject(Handle<JSObject> object) {
Maybe<bool> result =
delegate_->WriteHostObject(v8_isolate, Utils::ToLocal(object));
RETURN_VALUE_IF_SCHEDULED_EXCEPTION(isolate_, Nothing<bool>());
+ USE(result);
DCHECK(!result.IsNothing());
- return result;
+ DCHECK(result.ToChecked());
+ return ThrowIfOutOfMemory();
}
Maybe<uint32_t> ValueSerializer::WriteJSObjectPropertiesSlow(
diff --git a/test/unittests/value-serializer-unittest.cc b/test/unittests/value-serializer-unittest.cc
index 92603b588ad13480460a166b556be18e57fc8ad6..bc738c84fc8b15bdfcdb0cc995f8002362292a4c 100644
--- a/test/unittests/value-serializer-unittest.cc
+++ b/test/unittests/value-serializer-unittest.cc
@@ -119,7 +119,10 @@ class ValueSerializerTest : public TestWithIsolate {
}
std::pair<uint8_t*, size_t> buffer = serializer.Release();
std::vector<uint8_t> result(buffer.first, buffer.first + buffer.second);
- free(buffer.first);
+ if (auto* delegate = GetSerializerDelegate())
+ delegate->FreeBufferMemory(buffer.first);
+ else
+ free(buffer.first);
return Just(std::move(result));
}
@@ -138,6 +141,10 @@ class ValueSerializerTest : public TestWithIsolate {
return buffer;
}
+ std::vector<uint8_t> EncodeTest(const char* source) {
+ return EncodeTest(EvaluateScriptForInput(source));
+ }
+
v8::Local<v8::Message> InvalidEncodeTest(Local<Value> input_value) {
Context::Scope scope(serialization_context());
TryCatch try_catch(isolate());
@@ -2698,5 +2705,89 @@ TEST_F(ValueSerializerTestWithWasm, DecodeWasmModuleWithInvalidDataLength) {
InvalidDecodeTest({0xFF, 0x09, 0x3F, 0x00, 0x57, 0x79, 0x00, 0x7F});
}
+class ValueSerializerTestWithLimitedMemory : public ValueSerializerTest {
+ protected:
+// GMock doesn't use the "override" keyword.
+#if __clang__
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Winconsistent-missing-override"
+#endif
+
+ class SerializerDelegate : public ValueSerializer::Delegate {
+ public:
+ explicit SerializerDelegate(ValueSerializerTestWithLimitedMemory* test)
+ : test_(test) {}
+
+ ~SerializerDelegate() { EXPECT_EQ(nullptr, last_buffer_); }
+
+ void SetMemoryLimit(size_t limit) { memory_limit_ = limit; }
+
+ void* ReallocateBufferMemory(void* old_buffer, size_t size,
+ size_t* actual_size) override {
+ EXPECT_EQ(old_buffer, last_buffer_);
+ if (size > memory_limit_) return nullptr;
+ *actual_size = size;
+ last_buffer_ = realloc(old_buffer, size);
+ return last_buffer_;
+ }
+
+ void FreeBufferMemory(void* buffer) override {
+ EXPECT_EQ(buffer, last_buffer_);
+ last_buffer_ = nullptr;
+ free(buffer);
+ }
+
+ void ThrowDataCloneError(Local<String> message) override {
+ test_->isolate()->ThrowException(Exception::Error(message));
+ }
+
+ MOCK_METHOD2(WriteHostObject,
+ Maybe<bool>(Isolate* isolate, Local<Object> object));
+
+ private:
+ ValueSerializerTestWithLimitedMemory* test_;
+ void* last_buffer_ = nullptr;
+ size_t memory_limit_ = 0;
+ };
+
+#if __clang__
+#pragma clang diagnostic pop
+#endif
+
+ ValueSerializer::Delegate* GetSerializerDelegate() override {
+ return &serializer_delegate_;
+ }
+
+ void BeforeEncode(ValueSerializer* serializer) override {
+ serializer_ = serializer;
+ }
+
+ SerializerDelegate serializer_delegate_{this};
+ ValueSerializer* serializer_ = nullptr;
+};
+
+TEST_F(ValueSerializerTestWithLimitedMemory, FailIfNoMemoryInWriteHostObject) {
+ EXPECT_CALL(serializer_delegate_, WriteHostObject(isolate(), _))
+ .WillRepeatedly(Invoke([this](Isolate*, Local<Object>) {
+ static const char kDummyData[1024] = {};
+ serializer_->WriteRawBytes(&kDummyData, sizeof(kDummyData));
+ return Just(true);
+ }));
+
+ // If there is enough memory, things work.
+ serializer_delegate_.SetMemoryLimit(2048);
+ EncodeTest("new ExampleHostObject()");
+
+ // If not, we get a graceful failure, rather than silent misbehavior.
+ serializer_delegate_.SetMemoryLimit(1024);
+ InvalidEncodeTest("new ExampleHostObject()");
+
+ // And we definitely don't continue to serialize other things.
+ serializer_delegate_.SetMemoryLimit(1024);
+ EvaluateScriptForInput("gotA = false");
+ InvalidEncodeTest("[new ExampleHostObject, {get a() { gotA = true; }}]");
+ EXPECT_TRUE(EvaluateScriptForInput("gotA")->IsFalse());
+}
+
} // namespace
} // namespace v8