Modified: trunk/Source/WTF/ChangeLog (259787 => 259788)
--- trunk/Source/WTF/ChangeLog 2020-04-09 10:05:58 UTC (rev 259787)
+++ trunk/Source/WTF/ChangeLog 2020-04-09 10:52:52 UTC (rev 259788)
@@ -1,3 +1,20 @@
+2020-04-09 David Kilzer <ddkil...@apple.com>
+
+ Follow-up: WTF::Persistence::VectorCoder and IPC::VectorArgumentCoder should do bounds checking without crashing
+ <https://webkit.org/b/210227>
+ <rdar://problem/60832243>
+
+ Reviewed by Alex Christensen.
+
+ * wtf/persistence/PersistentCoders.h:
+ (WTF::Persistence::VectorCoder::decode):
+ - Replace safeCast<size_t> with isInBounds<size_t> so that we
+ don't crash if `decodedSize` is too big. Instead we fail
+ decoding by returning early.
+ - Revert checked arithemtic for multiplication since
+ bufferIsLargeEnoughToContain<T(size) already did this check
+ for us.
+
2020-04-09 Mark Lam <mark....@apple.com>
Implement a more efficient tagCFunction() tool.
Modified: trunk/Source/WTF/wtf/persistence/PersistentCoders.h (259787 => 259788)
--- trunk/Source/WTF/wtf/persistence/PersistentCoders.h 2020-04-09 10:05:58 UTC (rev 259787)
+++ trunk/Source/WTF/wtf/persistence/PersistentCoders.h 2020-04-09 10:52:52 UTC (rev 259788)
@@ -160,8 +160,11 @@
if (!decoder.decode(decodedSize))
return false;
- auto size = safeCast<size_t>(decodedSize);
+ if (!isInBounds<size_t>(decodedSize))
+ return false;
+ auto size = static_cast<size_t>(decodedSize);
+
// Since we know the total size of the elements, we can allocate the vector in
// one fell swoop. Before allocating we must however make sure that the decoder buffer
// is big enough.
@@ -171,8 +174,7 @@
Vector<T, inlineCapacity> temp;
temp.grow(size);
- Checked<size_t> checkedSize(size);
- decoder.decodeFixedLengthData(reinterpret_cast<uint8_t*>(temp.data()), (checkedSize * sizeof(T)).unsafeGet());
+ decoder.decodeFixedLengthData(reinterpret_cast<uint8_t*>(temp.data()), size * sizeof(T));
vector.swap(temp);
return true;
Modified: trunk/Source/WebKit/ChangeLog (259787 => 259788)
--- trunk/Source/WebKit/ChangeLog 2020-04-09 10:05:58 UTC (rev 259787)
+++ trunk/Source/WebKit/ChangeLog 2020-04-09 10:52:52 UTC (rev 259788)
@@ -1,3 +1,21 @@
+2020-04-09 David Kilzer <ddkil...@apple.com>
+
+ Follow-up: WTF::Persistence::VectorCoder and IPC::VectorArgumentCoder should do bounds checking without crashing
+ <https://webkit.org/b/210227>
+ <rdar://problem/60832243>
+
+ Reviewed by Alex Christensen.
+
+ * Platform/IPC/ArgumentCoders.h:
+ - Add missing call to decoder.markInvalid() if decoding of
+ `decodedSize` fails.
+ - Replace safeCast<size_t> with isInBounds<size_t> so that we
+ don't crash if `decodedSize` is too big. Instead we fail
+ decoding by marking the decoder invalid and returning early.
+ - Revert checked arithemtic for multiplication since
+ bufferIsLargeEnoughToContain<T(size) already did this check
+ for us.
+
2020-04-08 David Kilzer <ddkil...@apple.com>
WTF::Persistence::VectorCoder and IPC::VectorArgumentCoder should use checked arithmetic
Modified: trunk/Source/WebKit/Platform/IPC/ArgumentCoders.h (259787 => 259788)
--- trunk/Source/WebKit/Platform/IPC/ArgumentCoders.h 2020-04-09 10:05:58 UTC (rev 259787)
+++ trunk/Source/WebKit/Platform/IPC/ArgumentCoders.h 2020-04-09 10:52:52 UTC (rev 259788)
@@ -367,11 +367,18 @@
static bool decode(Decoder& decoder, Vector<T, inlineCapacity, OverflowHandler, minCapacity>& vector)
{
uint64_t decodedSize;
- if (!decoder.decode(decodedSize))
+ if (!decoder.decode(decodedSize)) {
+ decoder.markInvalid();
return false;
+ }
- auto size = safeCast<size_t>(decodedSize);
+ if (!WTF::isInBounds<size_t>(decodedSize)) {
+ decoder.markInvalid();
+ return false;
+ }
+ auto size = static_cast<size_t>(decodedSize);
+
// Since we know the total size of the elements, we can allocate the vector in
// one fell swoop. Before allocating we must however make sure that the decoder buffer
// is big enough.
@@ -383,8 +390,7 @@
Vector<T, inlineCapacity, OverflowHandler, minCapacity> temp;
temp.grow(size);
- Checked<size_t> checkedSize(size);
- if (!decoder.decodeFixedLengthData(reinterpret_cast<uint8_t*>(temp.data()), (checkedSize * sizeof(T)).unsafeGet(), alignof(T))) {
+ if (!decoder.decodeFixedLengthData(reinterpret_cast<uint8_t*>(temp.data()), size * sizeof(T), alignof(T))) {
decoder.markInvalid();
return false;
}
@@ -396,11 +402,18 @@
static Optional<Vector<T, inlineCapacity, OverflowHandler, minCapacity>> decode(Decoder& decoder)
{
uint64_t decodedSize;
- if (!decoder.decode(decodedSize))
+ if (!decoder.decode(decodedSize)) {
+ decoder.markInvalid();
return WTF::nullopt;
+ }
- auto size = safeCast<size_t>(decodedSize);
+ if (!WTF::isInBounds<size_t>(decodedSize)) {
+ decoder.markInvalid();
+ return WTF::nullopt;
+ }
+ auto size = static_cast<size_t>(decodedSize);
+
// Since we know the total size of the elements, we can allocate the vector in
// one fell swoop. Before allocating we must however make sure that the decoder buffer
// is big enough.
@@ -412,8 +425,7 @@
Vector<T, inlineCapacity, OverflowHandler, minCapacity> vector;
vector.grow(size);
- Checked<size_t> checkedSize(size);
- if (!decoder.decodeFixedLengthData(reinterpret_cast<uint8_t*>(vector.data()), (checkedSize * sizeof(T)).unsafeGet(), alignof(T))) {
+ if (!decoder.decodeFixedLengthData(reinterpret_cast<uint8_t*>(vector.data()), size * sizeof(T), alignof(T))) {
decoder.markInvalid();
return WTF::nullopt;
}
