Skip to content

Commit

Permalink
buffer: throw range error before truncating write
Browse files Browse the repository at this point in the history
The check to determine whether `noAssert` was set to true and thus
whether RangeErrors should be thrown was happening after the write was
truncated to the available size of the buffer. These checks now occur in
the correct order.

Fixes: #5587
PR-URL: #5605
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
Matt Loring authored and jasnell committed Mar 22, 2016
1 parent cde81b6 commit d3c0d1b
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 3 deletions.
6 changes: 3 additions & 3 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -814,14 +814,14 @@ void WriteFloatGeneric(const FunctionCallbackInfo<Value>& args) {
size_t offset = args[2]->IntegerValue(env->context()).FromMaybe(0);

size_t memcpy_num = sizeof(T);
if (offset + sizeof(T) > ts_obj_length)
memcpy_num = ts_obj_length - offset;

if (should_assert) {
CHECK_NOT_OOB(offset + memcpy_num >= memcpy_num);
CHECK_NOT_OOB(offset + memcpy_num <= ts_obj_length);
}
CHECK_LE(offset + memcpy_num, ts_obj_length);

if (offset + memcpy_num > ts_obj_length)
memcpy_num = ts_obj_length - offset;

union NoAlias {
T val;
Expand Down
10 changes: 10 additions & 0 deletions test/parallel/test-buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1038,6 +1038,16 @@ assert.throws(function() {
Buffer(0xFFFFFFFFF);
}, RangeError);

// issue GH-5587
assert.throws(function() {
var buf = new Buffer(8);
buf.writeFloatLE(0, 5);
}, RangeError);
assert.throws(function() {
var buf = new Buffer(16);
buf.writeDoubleLE(0, 9);
}, RangeError);


// attempt to overflow buffers, similar to previous bug in array buffers
assert.throws(function() {
Expand Down

0 comments on commit d3c0d1b

Please sign in to comment.