dgram: copy the list in send

This commit fix a possible crash situation in dgram send().
A crash is possible if an array is passed, and then altered after the
send call, as the call to libuv is wrapped in process.nextTick().
It also avoid sending an empty array to libuv by allocating an empty
buffer. It also does some cleanup inside send() to increase readability.

It removes test flakyness by use common.mustCall and
common.platformTimeout. Fixes situations were some events were not
asserted to be emitted.

Fixes: https://github.com/nodejs/node/issues/6616
PR-URL: https://github.com/nodejs/node/pull/6804
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

1 files changed, 24 insertions, 16 deletions

diff --git a/lib/dgram.js b/lib/dgram.js
index e6e88b1a9a..e6cc169dc6 100644
--- a/lib/dgram.js
+++ b/lib/dgram.js
@@ -263,16 +263,20 @@ function sliceBuffer(buffer, offset, length) {
 }
 
 
-function fixBuffer(buffer) {
-  for (var i = 0, l = buffer.length; i < l; i++) {
-    var buf = buffer[i];
+function fixBufferList(list) {
+  const newlist = new Array(list.length);
+
+  for (var i = 0, l = list.length; i < l; i++) {
+    var buf = list[i];
     if (typeof buf === 'string')
-      buffer[i] = Buffer.from(buf);
+      newlist[i] = Buffer.from(buf);
     else if (!(buf instanceof Buffer))
-      return false;
+      return null;
+    else
+      newlist[i] = buf;
   }
 
-  return true;
+  return newlist;
 }
 
 
@@ -306,7 +310,8 @@ Socket.prototype.send = function(buffer,
                                  port,
                                  address,
                                  callback) {
-  var self = this;
+  const self = this;
+  let list;
 
   if (address || (port && typeof port !== 'function')) {
     buffer = sliceBuffer(buffer, offset, length);
@@ -318,13 +323,13 @@ Socket.prototype.send = function(buffer,
 
   if (!Array.isArray(buffer)) {
     if (typeof buffer === 'string') {
-      buffer = [ Buffer.from(buffer) ];
+      list = [ Buffer.from(buffer) ];
     } else if (!(buffer instanceof Buffer)) {
       throw new TypeError('First argument must be a buffer or a string');
     } else {
-      buffer = [ buffer ];
+      list = [ buffer ];
     }
-  } else if (!fixBuffer(buffer)) {
+  } else if (!(list = fixBufferList(buffer))) {
     throw new TypeError('Buffer list arguments must be buffers or strings');
   }
 
@@ -342,20 +347,23 @@ Socket.prototype.send = function(buffer,
   if (self._bindState == BIND_STATE_UNBOUND)
     self.bind({port: 0, exclusive: true}, null);
 
+  if (list.length === 0)
+    list.push(Buffer.allocUnsafe(0));
+
   // If the socket hasn't been bound yet, push the outbound packet onto the
   // send queue and send after binding is complete.
   if (self._bindState != BIND_STATE_BOUND) {
-    enqueue(self, [buffer, port, address, callback]);
+    enqueue(self, [list, port, address, callback]);
     return;
   }
 
   self._handle.lookup(address, function afterDns(ex, ip) {
-    doSend(ex, self, ip, buffer, address, port, callback);
+    doSend(ex, self, ip, list, address, port, callback);
   });
 };
 
 
-function doSend(ex, self, ip, buffer, address, port, callback) {
+function doSend(ex, self, ip, list, address, port, callback) {
   if (ex) {
     if (typeof callback === 'function') {
       callback(ex);
@@ -369,7 +377,7 @@ function doSend(ex, self, ip, buffer, address, port, callback) {
   }
 
   var req = new SendWrap();
-  req.buffer = buffer;  // Keep reference alive.
+  req.list = list;  // Keep reference alive.
   req.address = address;
   req.port = port;
   if (callback) {
@@ -377,8 +385,8 @@ function doSend(ex, self, ip, buffer, address, port, callback) {
     req.oncomplete = afterSend;
   }
   var err = self._handle.send(req,
-                              buffer,
-                              buffer.length,
+                              list,
+                              list.length,
                               port,
                               ip,
                               !!callback);