cbor-sync
cbor-sync copied to clipboard
Fixes a DoS exploit in the decoder
Adds a check before trying to allocate an unbounded length buffer, which could be used to drain all the free memory from a server.
The issue also lies in the decoding of map/array lengths, causes crashes like:
<--- Last few GCs --->
[769:0x4d0ea20] 236975 ms: Scavenge 890.4 (907.8) -> 874.4 (907.8) MB, 1.1 / 0.0 ms (average mu = 0.723, current mu = 0.687) allocation failure
[769:0x4d0ea20] 236984 ms: Scavenge 890.4 (907.8) -> 874.4 (907.8) MB, 1.2 / 0.0 ms (average mu = 0.723, current mu = 0.687) allocation failure
[769:0x4d0ea20] 236992 ms: Scavenge 890.4 (907.8) -> 874.4 (907.8) MB, 0.8 / 0.0 ms (average mu = 0.723, current mu = 0.687) allocation failure
<--- JS stacktrace --->
FATAL ERROR: invalid array length Allocation failed - JavaScript heap out of memory
1: 0xa03530 node::Abort() [/usr/local/bin/node]
2: 0x94e471 node::FatalError(char const*, char const*) [/usr/local/bin/node]
3: 0xb7773e v8::Utils::ReportOOMFailure(v8::internal::Isolate*, char const*, bool) [/usr/local/bin/node]
4: 0xb77ab7 v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, bool) [/usr/local/bin/node]
5: 0xd32345 [/usr/local/bin/node]
6: 0xd0ae85 [/usr/local/bin/node]
7: 0xe9469e [/usr/local/bin/node]
8: 0xe947ed [/usr/local/bin/node]
9: 0x1030b03 v8::internal::Runtime_GrowArrayElements(int, unsigned long*, v8::internal::Isolate*) [/usr/local/bin/node]
10: 0x13ff179 [/usr/local/bin/node]
Can repro by:
const CBOR = require('cbor-sync');
CBOR.decode(Buffer.from('BAFFFFFFFF', 'hex'));
I'm not sure I'm meant to still have access to an mbed repo, but looks good to me 😛
Happy to merge once it's no longer a WiP
A fix turned out to be more complex than the one in this PR, the issue is here:
https://github.com/ARMmbed/cbor-sync/blob/35fdf44ce469ec34b7985f8463c806876fdb8d31/main.js#L223-L232
The array length is trusted and the loop continues to fill the result
array even when the input buffer has been used up, for stupidly long lengths this uses up all the memory. I'd guess the right fix would be to check if this.pos
in the BufferReader
overflows the input buffer length whenever it's updated, and throw an exception at that point.
In light of that, we switched out to a different decoder, but hopefully the debugging so far will be useful for someone motivated to fix it properly!
Found some time to do a better fix – @geraintluff might be worth a sanity check!
Looks like this fixes the DoS exploit. Could @geraintluff review it and release if you still have access?