src: refactor buffer bounds checking

Consolidate buffer bounds checking logic into Buffer namespace and use
it consistently throughout the source.

5 files changed, 23 insertions, 9 deletions

diff --git a/src/node_buffer.h b/src/node_buffer.h
index 7b7cf8e586..cb401b3b72 100644
--- a/src/node_buffer.h
+++ b/src/node_buffer.h
@@ -93,6 +93,20 @@ class NODE_EXTERN Buffer: public ObjectWrap {
     return Buffer::Length(b->handle_);
   }
 
+  // This is verbose to be explicit with inline commenting
+  static inline bool IsWithinBounds(size_t off, size_t len, size_t max) {
+    // Asking to seek too far into the buffer
+    // check to avoid wrapping in subsequent subtraction
+    if (off > max)
+      return false;
+
+    // Asking for more than is left over in the buffer
+    if (max - off < len)
+      return false;
+
+    // Otherwise we're in bounds
+    return true;
+  }
 
   ~Buffer();
 
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 9a57b167a7..e23f1502ce 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -1320,7 +1320,7 @@ Handle<Value> Connection::EncIn(const Arguments& args) {
 
   size_t off = args[1]->Int32Value();
   size_t len = args[2]->Int32Value();
-  if (off + len > buffer_length) {
+  if (!Buffer::IsWithinBounds(off, len, buffer_length)) {
     return ThrowException(Exception::Error(
           String::New("off + len > buffer.length")));
   }
@@ -1361,7 +1361,7 @@ Handle<Value> Connection::ClearOut(const Arguments& args) {
 
   size_t off = args[1]->Int32Value();
   size_t len = args[2]->Int32Value();
-  if (off + len > buffer_length) {
+  if (!Buffer::IsWithinBounds(off, len, buffer_length)) {
     return ThrowException(Exception::Error(
           String::New("off + len > buffer.length")));
   }
@@ -1437,7 +1437,7 @@ Handle<Value> Connection::EncOut(const Arguments& args) {
 
   size_t off = args[1]->Int32Value();
   size_t len = args[2]->Int32Value();
-  if (off + len > buffer_length) {
+  if (!Buffer::IsWithinBounds(off, len, buffer_length)) {
     return ThrowException(Exception::Error(
           String::New("off + len > buffer.length")));
   }
@@ -1471,7 +1471,7 @@ Handle<Value> Connection::ClearIn(const Arguments& args) {
 
   size_t off = args[1]->Int32Value();
   size_t len = args[2]->Int32Value();
-  if (off + len > buffer_length) {
+  if (!Buffer::IsWithinBounds(off, len, buffer_length)) {
     return ThrowException(Exception::Error(
           String::New("off + len > buffer.length")));
   }
diff --git a/src/node_file.cc b/src/node_file.cc
index 469ccbaaea..f665b19366 100644
--- a/src/node_file.cc
+++ b/src/node_file.cc
@@ -733,7 +733,7 @@ static Handle<Value> Write(const Arguments& args) {
   }
 
   ssize_t len = args[3]->Int32Value();
-  if (off + len > buffer_length) {
+  if (!Buffer::IsWithinBounds(off, len, buffer_length)) {
     return ThrowException(Exception::Error(
           String::New("off + len > buffer.length")));
   }
@@ -796,7 +796,7 @@ static Handle<Value> Read(const Arguments& args) {
   }
 
   len = args[3]->Int32Value();
-  if (off + len > buffer_length) {
+  if (!Buffer::IsWithinBounds(off, len, buffer_length)) {
     return ThrowException(Exception::Error(
           String::New("Length extends beyond buffer")));
   }
diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc
index 47e229d1ed..0a261b003e 100644
--- a/src/node_http_parser.cc
+++ b/src/node_http_parser.cc
@@ -410,7 +410,7 @@ public:
     }
 
     size_t len = args[2]->Int32Value();
-    if (off+len > buffer_len) {
+    if (!Buffer::IsWithinBounds(off, len, buffer_len)) {
       return ThrowException(Exception::Error(
             String::New("off + len > buffer.length")));
     }
diff --git a/src/node_zlib.cc b/src/node_zlib.cc
index 9acbfcb75a..d522676284 100644
--- a/src/node_zlib.cc
+++ b/src/node_zlib.cc
@@ -155,7 +155,7 @@ class ZCtx : public ObjectWrap {
       in_off = args[2]->Uint32Value();
       in_len = args[3]->Uint32Value();
 
-      assert(in_off + in_len <= Buffer::Length(in_buf));
+      assert(Buffer::IsWithinBounds(in_off, in_len, Buffer::Length(in_buf)));
       in = reinterpret_cast<Bytef *>(Buffer::Data(in_buf) + in_off);
     }
 
@@ -163,7 +163,7 @@ class ZCtx : public ObjectWrap {
     Local<Object> out_buf = args[4]->ToObject();
     out_off = args[5]->Uint32Value();
     out_len = args[6]->Uint32Value();
-    assert(out_off + out_len <= Buffer::Length(out_buf));
+    assert(Buffer::IsWithinBounds(out_off, out_len, Buffer::Length(out_buf)));
     out = reinterpret_cast<Bytef *>(Buffer::Data(out_buf) + out_off);
 
     // build up the work request