Skip to content

Commit 5c832e4

Browse files
tjfontainetjfontaine
committed
src: refactor buffer bounds checking
Consolidate buffer bounds checking logic into Buffer namespace and use it consistently throughout the source.
1 parent 2e8bb57 commit 5c832e4

5 files changed

Lines changed: 23 additions & 9 deletions

File tree

‎src/node_buffer.h‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,20 @@ class NODE_EXTERN Buffer: public ObjectWrap {
9393
return Buffer::Length(b->handle_);
9494
}
9595

96+
// This is verbose to be explicit with inline commenting
97+
static inline bool IsWithinBounds(size_t off, size_t len, size_t max) {
98+
// Asking to seek too far into the buffer
99+
// check to avoid wrapping in subsequent subtraction
100+
if (off > max)
101+
return false;
102+
103+
// Asking for more than is left over in the buffer
104+
if (max - off < len)
105+
return false;
106+
107+
// Otherwise we're in bounds
108+
return true;
109+
}
96110

97111
~Buffer();
98112

‎src/node_crypto.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1320,7 +1320,7 @@ Handle<Value> Connection::EncIn(const Arguments& args) {
13201320

13211321
size_t off = args[1]->Int32Value();
13221322
size_t len = args[2]->Int32Value();
1323-
if (off + len > buffer_length) {
1323+
if (!Buffer::IsWithinBounds(off, len, buffer_length)) {
13241324
return ThrowException(Exception::Error(
13251325
String::New("off + len > buffer.length")));
13261326
}
@@ -1361,7 +1361,7 @@ Handle<Value> Connection::ClearOut(const Arguments& args) {
13611361

13621362
size_t off = args[1]->Int32Value();
13631363
size_t len = args[2]->Int32Value();
1364-
if (off + len > buffer_length) {
1364+
if (!Buffer::IsWithinBounds(off, len, buffer_length)) {
13651365
return ThrowException(Exception::Error(
13661366
String::New("off + len > buffer.length")));
13671367
}
@@ -1437,7 +1437,7 @@ Handle<Value> Connection::EncOut(const Arguments& args) {
14371437

14381438
size_t off = args[1]->Int32Value();
14391439
size_t len = args[2]->Int32Value();
1440-
if (off + len > buffer_length) {
1440+
if (!Buffer::IsWithinBounds(off, len, buffer_length)) {
14411441
return ThrowException(Exception::Error(
14421442
String::New("off + len > buffer.length")));
14431443
}
@@ -1471,7 +1471,7 @@ Handle<Value> Connection::ClearIn(const Arguments& args) {
14711471

14721472
size_t off = args[1]->Int32Value();
14731473
size_t len = args[2]->Int32Value();
1474-
if (off + len > buffer_length) {
1474+
if (!Buffer::IsWithinBounds(off, len, buffer_length)) {
14751475
return ThrowException(Exception::Error(
14761476
String::New("off + len > buffer.length")));
14771477
}

‎src/node_file.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -733,7 +733,7 @@ static Handle<Value> Write(const Arguments& args) {
733733
}
734734

735735
ssize_t len = args[3]->Int32Value();
736-
if (off + len > buffer_length) {
736+
if (!Buffer::IsWithinBounds(off, len, buffer_length)) {
737737
return ThrowException(Exception::Error(
738738
String::New("off + len > buffer.length")));
739739
}
@@ -796,7 +796,7 @@ static Handle<Value> Read(const Arguments& args) {
796796
}
797797

798798
len = args[3]->Int32Value();
799-
if (off + len > buffer_length) {
799+
if (!Buffer::IsWithinBounds(off, len, buffer_length)) {
800800
return ThrowException(Exception::Error(
801801
String::New("Length extends beyond buffer")));
802802
}

‎src/node_http_parser.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,7 @@ class Parser : public ObjectWrap {
410410
}
411411

412412
size_t len = args[2]->Int32Value();
413-
if (off+len > buffer_len) {
413+
if (!Buffer::IsWithinBounds(off, len, buffer_len)) {
414414
return ThrowException(Exception::Error(
415415
String::New("off + len > buffer.length")));
416416
}

‎src/node_zlib.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,15 +155,15 @@ class ZCtx : public ObjectWrap {
155155
in_off = args[2]->Uint32Value();
156156
in_len = args[3]->Uint32Value();
157157

158-
assert(in_off + in_len <= Buffer::Length(in_buf));
158+
assert(Buffer::IsWithinBounds(in_off, in_len, Buffer::Length(in_buf)));
159159
in = reinterpret_cast<Bytef *>(Buffer::Data(in_buf) + in_off);
160160
}
161161

162162
assert(Buffer::HasInstance(args[4]));
163163
Local<Object> out_buf = args[4]->ToObject();
164164
out_off = args[5]->Uint32Value();
165165
out_len = args[6]->Uint32Value();
166-
assert(out_off + out_len <= Buffer::Length(out_buf));
166+
assert(Buffer::IsWithinBounds(out_off, out_len, Buffer::Length(out_buf)));
167167
out = reinterpret_cast<Bytef *>(Buffer::Data(out_buf) + out_off);
168168

169169
// build up the work request

0 commit comments

Comments
 (0)