Skip to content

Commit 498200b

Browse files
committed
buffer: reject negative SlowBuffer offsets
Reject negative offsets in SlowBuffer::MakeFastBuffer(), it allows the creation of buffers that point to arbitrary addresses. Reported by Trevor Norris.
1 parent 6b4a935 commit 498200b

File tree

3 files changed

+59
-1
lines changed

3 files changed

+59
-1
lines changed

lib/buffer.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -557,7 +557,7 @@ Buffer.prototype.slice = function(start, end) {
557557
if (end === undefined) end = this.length;
558558
if (end > this.length) throw new Error('oob');
559559
if (start > end) throw new Error('oob');
560-
560+
if (start < 0) throw new Error('start out of bounds');
561561
return new Buffer(this.parent, end - start, +start + this.offset);
562562
};
563563

src/node_buffer.cc

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -708,6 +708,19 @@ Handle<Value> Buffer::MakeFastBuffer(const Arguments &args) {
708708
uint32_t offset = args[2]->Uint32Value();
709709
uint32_t length = args[3]->Uint32Value();
710710

711+
if (offset > buffer->length_) {
712+
return ThrowRangeError("offset out of range");
713+
}
714+
715+
if (offset + length > buffer->length_) {
716+
return ThrowRangeError("length out of range");
717+
}
718+
719+
// Check for wraparound. Safe because offset and length are unsigned.
720+
if (offset + length < offset) {
721+
return ThrowRangeError("offset or length out of range");
722+
}
723+
711724
fast_buffer->SetIndexedPropertiesToExternalArrayData(buffer->data_ + offset,
712725
kExternalUnsignedByteArray,
713726
length);

test/simple/test-buffer.js

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
var common = require('../common');
2323
var assert = require('assert');
2424

25+
var SlowBuffer = require('buffer').SlowBuffer;
2526
var Buffer = require('buffer').Buffer;
2627

2728
var b = Buffer(1024); // safe constructor
@@ -745,3 +746,47 @@ assert.throws(function() {
745746
assert.throws(function() {
746747
new Buffer(0xFFFFFFFFF);
747748
}, TypeError);
749+
750+
// SlowBuffer sanity checks.
751+
assert.throws(function() {
752+
var len = 0xfffff;
753+
var sbuf = new SlowBuffer(len);
754+
var buf = new Buffer(sbuf, len, 0);
755+
SlowBuffer.makeFastBuffer(sbuf, buf, -len, len); // Should throw.
756+
for (var i = 0; i < len; ++i) buf[i] = 0x42; // Try to force segfault.
757+
}, RangeError);
758+
759+
assert.throws(function() {
760+
var len = 0xfffff;
761+
var sbuf = new SlowBuffer(len);
762+
var buf = new Buffer(sbuf, len, -len); // Should throw.
763+
for (var i = 0; i < len; ++i) buf[i] = 0x42; // Try to force segfault.
764+
}, RangeError);
765+
766+
assert.throws(function() {
767+
var len = 0xfffff;
768+
var sbuf = new SlowBuffer(len);
769+
sbuf = sbuf.slice(-len); // Should throw.
770+
for (var i = 0; i < len; ++i) sbuf[i] = 0x42; // Try to force segfault.
771+
}, RangeError);
772+
773+
assert.throws(function() {
774+
var sbuf = new SlowBuffer(1);
775+
var buf = new Buffer(sbuf, 1, 0);
776+
buf.length = 0xffffffff;
777+
buf.slice(0xffffff0, 0xffffffe); // Should throw.
778+
}, Error);
779+
780+
assert.throws(function() {
781+
var sbuf = new SlowBuffer(8);
782+
var buf = new Buffer(sbuf, 8, 0);
783+
buf.slice(-8); // Should throw. Throws Error instead of RangeError
784+
// for the sake of v0.8 compatibility.
785+
}, Error);
786+
787+
assert.throws(function() {
788+
var sbuf = new SlowBuffer(16);
789+
var buf = new Buffer(sbuf, 8, 8);
790+
buf.slice(-8); // Should throw. Throws Error instead of RangeError
791+
// for the sake of v0.8 compatibility.
792+
}, Error);

0 commit comments

Comments
 (0)