From a22a2d8656419586647ca1aa6c5008f3e9045424 Mon Sep 17 00:00:00 2001
From: Maxwell Krohn <themax@gmail.com>
Date: Tue, 25 Feb 2014 15:48:31 -0500
Subject: [PATCH] tls: stop NodeBIO::Gets from reading off end of buffer

NodeBIO::Gets was reading off the end of a buffer if it
didn't find a "\n" before the EOF.  This behavior
was causing X509 certificates passed to `https.Agent`
via the "ca" option to be silently discarded. It also
was causing improper parsing of certs and keys
passed to https.Agent, but those problems were worked
around in cdde9a3.

Backed out workaround in `lib/crypto.js` from ccde9a3,
which now isn't needed.  But keep the test introduced
in that commit, which tests properly for this
bug.

This bug was first introduced in a58f93f

Gist containing test code, bisection log, and notes:
   https://gist.github.com/maxtaco/9211605
---
 lib/crypto.js          | 19 +++----------------
 src/node_crypto_bio.cc |  4 ++--
 2 files changed, 5 insertions(+), 18 deletions(-)

diff --git a/lib/crypto.js b/lib/crypto.js
index 3572e0a..bd3aedc 100644
--- a/lib/crypto.js
+++ b/lib/crypto.js
@@ -80,18 +80,6 @@ function Credentials(secureProtocol, flags, context) {
 
 exports.Credentials = Credentials;
 
-function addNewline(buf) {
-  var last = buf[buf.length - 1];
-  var isBuf = Buffer.isBuffer(buf);
-
-  if (!isBuf && !util.isString(buf))
-    throw new Error('Certificate should be of type Buffer or string');
-
-  if (isBuf ? last !== 10 : last !== '\n')
-    return buf.toString().trim() + '\n';
-  else
-    return buf;
-}
 
 exports.createCredentials = function(options, context) {
   if (!options) options = {};
@@ -103,15 +91,14 @@ exports.createCredentials = function(options, context) {
   if (context) return c;
 
   if (options.key) {
-    var key = addNewline(options.key);
     if (options.passphrase) {
-      c.context.setKey(key, options.passphrase);
+      c.context.setKey(options.key, options.passphrase);
     } else {
-      c.context.setKey(key);
+      c.context.setKey(options.key);
     }
   }
 
-  if (options.cert) c.context.setCert(addNewline(options.cert));
+  if (options.cert) c.context.setCert(options.cert);
 
   if (options.ciphers) c.context.setCiphers(options.ciphers);
 
diff --git a/src/node_crypto_bio.cc b/src/node_crypto_bio.cc
index eaf8f76..22f0f6e 100644
--- a/src/node_crypto_bio.cc
+++ b/src/node_crypto_bio.cc
@@ -145,8 +145,8 @@ int NodeBIO::Gets(BIO* bio, char* out, int size) {
 
   int i = nbio->IndexOf('\n', size);
 
-  // Include '\n'
-  if (i < size)
+  // Include '\n', if it's there.  If not, don't read off the end.
+  if (i < size && i >= 0 && static_cast<size_t>(i) < nbio->Length())
     i++;
 
   // Shift `i` a bit to NULL-terminate string later
-- 
2.7.4