Skip to content

Commit

Permalink
buffer: reject negative SlowBuffer offsets
Browse files Browse the repository at this point in the history
Reject negative offsets in SlowBuffer::MakeFastBuffer(), it allows
the creation of buffers that point to arbitrary addresses.

Reported by Trevor Norris.
  • Loading branch information
bnoordhuis committed Jan 17, 2013
1 parent 6b4a935 commit 498200b
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,7 @@ Buffer.prototype.slice = function(start, end) {
if (end === undefined) end = this.length;
if (end > this.length) throw new Error('oob');
if (start > end) throw new Error('oob');

if (start < 0) throw new Error('start out of bounds');
return new Buffer(this.parent, end - start, +start + this.offset);
};

Expand Down
13 changes: 13 additions & 0 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,19 @@ Handle<Value> Buffer::MakeFastBuffer(const Arguments &args) {
uint32_t offset = args[2]->Uint32Value();
uint32_t length = args[3]->Uint32Value();

if (offset > buffer->length_) {
return ThrowRangeError("offset out of range");
}

if (offset + length > buffer->length_) {
return ThrowRangeError("length out of range");
}

// Check for wraparound. Safe because offset and length are unsigned.
if (offset + length < offset) {
return ThrowRangeError("offset or length out of range");
}

fast_buffer->SetIndexedPropertiesToExternalArrayData(buffer->data_ + offset,
kExternalUnsignedByteArray,
length);
Expand Down
45 changes: 45 additions & 0 deletions test/simple/test-buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
var common = require('../common');
var assert = require('assert');

var SlowBuffer = require('buffer').SlowBuffer;
var Buffer = require('buffer').Buffer;

var b = Buffer(1024); // safe constructor
Expand Down Expand Up @@ -745,3 +746,47 @@ assert.throws(function() {
assert.throws(function() {
new Buffer(0xFFFFFFFFF);
}, TypeError);

// SlowBuffer sanity checks.
assert.throws(function() {
var len = 0xfffff;
var sbuf = new SlowBuffer(len);
var buf = new Buffer(sbuf, len, 0);
SlowBuffer.makeFastBuffer(sbuf, buf, -len, len); // Should throw.
for (var i = 0; i < len; ++i) buf[i] = 0x42; // Try to force segfault.
}, RangeError);

assert.throws(function() {
var len = 0xfffff;
var sbuf = new SlowBuffer(len);
var buf = new Buffer(sbuf, len, -len); // Should throw.
for (var i = 0; i < len; ++i) buf[i] = 0x42; // Try to force segfault.
}, RangeError);

assert.throws(function() {
var len = 0xfffff;
var sbuf = new SlowBuffer(len);
sbuf = sbuf.slice(-len); // Should throw.
for (var i = 0; i < len; ++i) sbuf[i] = 0x42; // Try to force segfault.
}, RangeError);

assert.throws(function() {
var sbuf = new SlowBuffer(1);
var buf = new Buffer(sbuf, 1, 0);
buf.length = 0xffffffff;
buf.slice(0xffffff0, 0xffffffe); // Should throw.
}, Error);

assert.throws(function() {
var sbuf = new SlowBuffer(8);
var buf = new Buffer(sbuf, 8, 0);
buf.slice(-8); // Should throw. Throws Error instead of RangeError
// for the sake of v0.8 compatibility.
}, Error);

assert.throws(function() {
var sbuf = new SlowBuffer(16);
var buf = new Buffer(sbuf, 8, 8);
buf.slice(-8); // Should throw. Throws Error instead of RangeError
// for the sake of v0.8 compatibility.
}, Error);

0 comments on commit 498200b

Please sign in to comment.