Skip to content
This repository was archived by the owner on Apr 22, 2023. It is now read-only.

Commit

Permalink
tls: remove harmful unnecessary bounds checking
Browse files Browse the repository at this point in the history
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) { ...`

448e0f4 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.
  • Loading branch information
laverdet authored and indutny committed Mar 23, 2013
1 parent f7ebb4d commit 9430ca6
Showing 1 changed file with 0 additions and 20 deletions.
20 changes: 0 additions & 20 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1313,11 +1313,6 @@ Handle<Value> 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(
Expand Down Expand Up @@ -1359,11 +1354,6 @@ Handle<Value> 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(
Expand Down Expand Up @@ -1431,11 +1421,6 @@ Handle<Value> 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(
Expand Down Expand Up @@ -1470,11 +1455,6 @@ Handle<Value> 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(
Expand Down

0 comments on commit 9430ca6

Please sign in to comment.