From 0cff0521c333df8649bc6a2b1f40adbf899b1261 Mon Sep 17 00:00:00 2001
From: cjihrig <cjihrig@gmail.com>
Date: Tue, 16 Dec 2014 17:17:28 -0500
Subject: [PATCH] net: throw on invalid socket timeouts

This commit restricts socket timeouts non-negative, finite
numbers. Any other value throws a TypeError or RangeError.
This prevents subtle bugs that can happen due to type
coercion.

Fixes: https://github.com/joyent/node/issues/8618
PR-URL: https://github.com/joyent/node/pull/8884
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Timothy J Fontaine <tjfontaine@gmail.com>

Conflicts:
	lib/timers.js
	test/simple/test-net-settimeout.js
	test/simple/test-net-socket-timeout.js
---
 lib/net.js                               | 12 ++++++------
 lib/timers.js                            | 18 +++++++++++------
 test/parallel/test-net-settimeout.js     |  2 +-
 test/parallel/test-net-socket-timeout.js | 25 ++++++++++++++++++++++++
 4 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/lib/net.js b/lib/net.js
index 030083d3f40983..294c7f99ee6a7e 100644
--- a/lib/net.js
+++ b/lib/net.js
@@ -300,17 +300,17 @@ Socket.prototype.listen = function() {
 
 
 Socket.prototype.setTimeout = function(msecs, callback) {
-  if (msecs > 0 && isFinite(msecs)) {
+  if (msecs === 0) {
+    timers.unenroll(this);
+    if (callback) {
+      this.removeListener('timeout', callback);
+    }
+  } else {
     timers.enroll(this, msecs);
     timers._unrefActive(this);
     if (callback) {
       this.once('timeout', callback);
     }
-  } else if (msecs === 0) {
-    timers.unenroll(this);
-    if (callback) {
-      this.removeListener('timeout', callback);
-    }
   }
 };
 
diff --git a/lib/timers.js b/lib/timers.js
index 4dc483ce8be1a8..e3e64287509a30 100644
--- a/lib/timers.js
+++ b/lib/timers.js
@@ -3,15 +3,13 @@
 const Timer = process.binding('timer_wrap').Timer;
 const L = require('_linklist');
 const assert = require('assert').ok;
-
+const util = require('util');
+const debug = util.debuglog('timer');
 const kOnTimeout = Timer.kOnTimeout | 0;
 
 // Timeout values > TIMEOUT_MAX are set to 1.
 const TIMEOUT_MAX = 2147483647; // 2^31-1
 
-const debug = require('util').debuglog('timer');
-
-
 // IDLE TIMEOUTS
 //
 // Because often many sockets will have the same idle timeout we will not
@@ -132,13 +130,21 @@ const unenroll = exports.unenroll = function(item) {
 
 // Does not start the time, just sets up the members needed.
 exports.enroll = function(item, msecs) {
+  if (typeof msecs !== 'number') {
+    throw new TypeError('msecs must be a number');
+  }
+
+  if (msecs < 0 || !isFinite(msecs)) {
+    throw new RangeError('msecs must be a non-negative finite number');
+  }
+
   // if this item was already in a list somewhere
   // then we should unenroll it from that
   if (item._idleNext) unenroll(item);
 
   // Ensure that msecs fits into signed int32
-  if (msecs > 0x7fffffff) {
-    msecs = 0x7fffffff;
+  if (msecs > TIMEOUT_MAX) {
+    msecs = TIMEOUT_MAX;
   }
 
   item._idleTimeout = msecs;
diff --git a/test/parallel/test-net-settimeout.js b/test/parallel/test-net-settimeout.js
index 3bf26974576f2b..28943a1ef0c96b 100644
--- a/test/parallel/test-net-settimeout.js
+++ b/test/parallel/test-net-settimeout.js
@@ -12,7 +12,7 @@ var server = net.createServer(function(c) {
 });
 server.listen(common.PORT);
 
-var killers = [0, Infinity, NaN];
+var killers = [0];
 
 var left = killers.length;
 killers.forEach(function(killer) {
diff --git a/test/parallel/test-net-socket-timeout.js b/test/parallel/test-net-socket-timeout.js
index c4d84fa177f51b..f4038298763e8d 100644
--- a/test/parallel/test-net-socket-timeout.js
+++ b/test/parallel/test-net-socket-timeout.js
@@ -2,6 +2,31 @@ var common = require('../common');
 var net = require('net');
 var assert = require('assert');
 
+// Verify that invalid delays throw
+var noop = function() {};
+var s = new net.Socket();
+var nonNumericDelays = ['100', true, false, undefined, null, '', {}, noop, []];
+var badRangeDelays = [-0.001, -1, -Infinity, Infinity, NaN];
+var validDelays = [0, 0.001, 1, 1e6];
+
+for (var i = 0; i < nonNumericDelays.length; i++) {
+  assert.throws(function() {
+    s.setTimeout(nonNumericDelays[i], noop);
+  }, TypeError);
+}
+
+for (var i = 0; i < badRangeDelays.length; i++) {
+  assert.throws(function() {
+    s.setTimeout(badRangeDelays[i], noop);
+  }, RangeError);
+}
+
+for (var i = 0; i < validDelays.length; i++) {
+  assert.doesNotThrow(function() {
+    s.setTimeout(validDelays[i], noop);
+  });
+}
+
 var timedout = false;
 
 var server = net.Server();