3 ms·
I suggest that the most expedient (cheapest) language to migrate the existing code base to would be a memory safe subset of C++ [1]. In practice most of the saf
by duneroadrunner 8y ago
I suggest that the most expedient (cheapest) language to migrate the existing code base to would be a memory safe subset of C++ [1]. In practice most of the safety benefit could be obtained from a just a partial migration. Specifically, just banning raw pointers/views/spans and non-bounds-checked arrays and vectors. From a quick glance at the code in the (quite small) patch diff, the code in question includes:
DOMArrayBuffer* result = DOMArrayBuffer::Create(raw_data_->ToArrayBuffer());
...
raw_data_.reset();
...
return result;
I'd imagine the effort/time/money it would take to replace those raw pointers with memory safe substitutes [2][3][4] (and enforce the ban going forward) would be relatively modest. Performance shouldn't be an issue [5].
[1] shameless plug: https://github.com/duneroadrunner/SaferCPlusPlus https://github.com/duneroadrunner/SaferCPlusPlus
[2] https://github.com/duneroadrunner/SaferCPlusPlus#registered-pointers https://github.com/duneroadrunner/SaferCPlusPlus#registered-...
[3] https://github.com/duneroadrunner/SaferCPlusPlus#norad-pointers https://github.com/duneroadrunner/SaferCPlusPlus#norad-point...
[4] https://github.com/duneroadrunner/SaferCPlusPlus#scope-pointers https://github.com/duneroadrunner/SaferCPlusPlus#scope-point...
[5] https://github.com/duneroadrunner/SaferCPlusPlus-BenchmarksGame https://github.com/duneroadrunner/SaferCPlusPlus-BenchmarksG...
- duneroadrunner 8y agoUg. After closer inspection, it looks like those particular raw pointers seem to be managed by a garbage collector. (Specifically, the "Blink GC" [1].) As others have pointed out, this particular bug may not actually be a C++ issue. (Or at least not a typical one.) [1] https://chromium.googlesource.com/chromium/src/+/master/third_party/blink/renderer/platform/heap/BlinkGCDesign.md https://chromium.googlesource.com/chromium/src/+/master/thir...
- ErikCorry 8y agoNot so sure. I haven't worked on this code for a while and have no non-public knowledge of the bug, but ArrayBufferBuilder does not inherit from any of the GCed base classes, and has the USING_FAST_MALLOC macro which is used for non-GC classes. https://chromium.googlesource.com/chromium/src/+/refs/heads/master/third_party/blink/renderer/platform/wtf/typed_arrays/array_buffer_builder.h https://chromium.googlesource.com/chromium/src/+/refs/heads/... The ArrayBuffer itself also uses reference counters rather than the GC base classes from Oilpan. https://chromium.googlesource.com/chromium/src/+/refs/heads/master/third_party/blink/renderer/platform/wtf/typed_arrays/array_buffer.h https://chromium.googlesource.com/chromium/src/+/refs/heads/... See also https://chromium.googlesource.com/chromium/src/+/refs/heads/master/third_party/blink/renderer/platform/wtf/Allocator.md https://chromium.googlesource.com/chromium/src/+/refs/heads/... and https://chromium.googlesource.com/chromium/src/+/refs/heads/master/third_party/blink/renderer/platform/heap/BlinkGCAPIReference.md https://chromium.googlesource.com/chromium/src/+/refs/heads/...
- duneroadrunner 8y agoArrayBufferBuilder isn't, but DOMArrayBuffer seems to be a GC managed type [1], right? And, before the patch, the DOMArrayBuffer held a "refcounting pointer" potentially targeting raw_data_'s reference counted ArrayBuffer, right? I don't see any immediately apparent use-after-free with this, so I assume raw_data_'s ArrayBuffer is being messed with elsewhere? As someone who worked on the code, do you have an idea/hunch about where the invalid memory access actually occurs? https://github.com/chromium/chromium/blob/ba9748e78ec7e9c0d594e7edf7b2c07ea2a90449/third_party/blink/renderer/core/typed_arrays/dom_array_buffer.h#L20-L22 https://github.com/chromium/chromium/blob/ba9748e78ec7e9c0d5...
- ErikCorry 8y agoYes, DOMArrayBuffer inherits via ScriptWrappable from GarbageCollectedFinalized<> so it's on the GCed heap. I don't understand the UAF yet, I'm hoping someone will write a blog post on it later :-).