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 Fishrock123 committed Mar 22, 2016
1 parent 341b3d0 commit 72fb796
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 @@ -804,14 +804,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() {
new 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 72fb796

Please sign in to comment.