From 9430ca686504b6f178be512bf307f3e2a9eba6f0 Mon Sep 17 00:00:00 2001 From: Marcel Laverdet Date: Thu, 21 Mar 2013 16:56:02 -0500 Subject: [PATCH] tls: remove harmful unnecessary bounds checking The EncIn, EncOut, ClearIn & ClearOut functions are victims of some code copy + pasting. A common line copied to all of them is: `if (off >= buffer_length) { ...` 448e0f43 corrected ClearIn's check from `>=` to `>`, but left the others unchanged (with an incorrect bounds check). However, if you look down at the next very next bounds check you'll see: `if (off + len > buffer_length) { ...` So the check is actually obviated by the next line, and should be removed. This fixes an issue where writing a zero-length buffer to an encrypted pair's *encrypted* stream you would get a crash. --- src/node_crypto.cc | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index c9b3a5164f0..bab3b6971cf 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -1313,11 +1313,6 @@ Handle Connection::EncIn(const Arguments& args) { size_t buffer_length = Buffer::Length(args[0]); size_t off = args[1]->Int32Value(); - if (off >= buffer_length) { - return ThrowException(Exception::Error( - String::New("Offset is out of bounds"))); - } - size_t len = args[2]->Int32Value(); if (off + len > buffer_length) { return ThrowException(Exception::Error( @@ -1359,11 +1354,6 @@ Handle Connection::ClearOut(const Arguments& args) { size_t buffer_length = Buffer::Length(args[0]); size_t off = args[1]->Int32Value(); - if (off >= buffer_length) { - return ThrowException(Exception::Error( - String::New("Offset is out of bounds"))); - } - size_t len = args[2]->Int32Value(); if (off + len > buffer_length) { return ThrowException(Exception::Error( @@ -1431,11 +1421,6 @@ Handle Connection::EncOut(const Arguments& args) { size_t buffer_length = Buffer::Length(args[0]); size_t off = args[1]->Int32Value(); - if (off >= buffer_length) { - return ThrowException(Exception::Error( - String::New("Offset is out of bounds"))); - } - size_t len = args[2]->Int32Value(); if (off + len > buffer_length) { return ThrowException(Exception::Error( @@ -1470,11 +1455,6 @@ Handle Connection::ClearIn(const Arguments& args) { size_t buffer_length = Buffer::Length(args[0]); size_t off = args[1]->Int32Value(); - if (off > buffer_length) { - return ThrowException(Exception::Error( - String::New("Offset is out of bounds"))); - } - size_t len = args[2]->Int32Value(); if (off + len > buffer_length) { return ThrowException(Exception::Error(