From 72fb796bed06cb2bd78e871224c8375772af2308 Mon Sep 17 00:00:00 2001 From: Matt Loring Date: Tue, 8 Mar 2016 11:15:01 -0800 Subject: [PATCH] buffer: throw range error before truncating write 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: https://github.com/nodejs/node/issues/5587 PR-URL: https://github.com/nodejs/node/pull/5605 Reviewed-By: Trevor Norris Reviewed-By: James M Snell --- src/node_buffer.cc | 6 +++--- test/parallel/test-buffer.js | 10 ++++++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 026a04028a1eb8..1c60a311a531e8 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -804,14 +804,14 @@ void WriteFloatGeneric(const FunctionCallbackInfo& 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; diff --git a/test/parallel/test-buffer.js b/test/parallel/test-buffer.js index 387d57e5a95cf7..9d6d0cee2c60a5 100644 --- a/test/parallel/test-buffer.js +++ b/test/parallel/test-buffer.js @@ -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() {