From ddb12acdd13f7bb66eb03c61d449873ef4582dee Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Tue, 7 Nov 2017 19:28:06 +0800
Subject: [PATCH] src: make StreamBase prototype accessors more robust

---
 src/stream_base-inl.h                         | 15 ++++++++---
 ...-base-prototype-accessors-enumerability.js | 19 +++++++++++++
 .../test-stream-base-prototype-accessors.js   | 27 +++++++++++++++++++
 3 files changed, 57 insertions(+), 4 deletions(-)
 create mode 100644 test/parallel/test-stream-base-prototype-accessors-enumerability.js
 create mode 100644 test/parallel/test-stream-base-prototype-accessors.js

diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h
index e1e50802f2a06a..f75832393d1c8e 100644
--- a/src/stream_base-inl.h
+++ b/src/stream_base-inl.h
@@ -11,6 +11,7 @@
 
 namespace node {
 
+using v8::AccessorSignature;
 using v8::External;
 using v8::FunctionCallbackInfo;
 using v8::FunctionTemplate;
@@ -31,27 +32,33 @@ void StreamBase::AddMethods(Environment* env,
   HandleScope scope(env->isolate());
 
   enum PropertyAttribute attributes =
-      static_cast<PropertyAttribute>(v8::ReadOnly | v8::DontDelete);
+      static_cast<PropertyAttribute>(
+        v8::ReadOnly | v8::DontDelete | v8::DontEnum);
+  Local<AccessorSignature> signature =
+    AccessorSignature::New(env->isolate(), t);
   t->PrototypeTemplate()->SetAccessor(env->fd_string(),
                                       GetFD<Base>,
                                       nullptr,
                                       env->as_external(),
                                       v8::DEFAULT,
-                                      attributes);
+                                      attributes,
+                                      signature);
 
   t->PrototypeTemplate()->SetAccessor(env->external_stream_string(),
                                       GetExternal<Base>,
                                       nullptr,
                                       env->as_external(),
                                       v8::DEFAULT,
-                                      attributes);
+                                      attributes,
+                                      signature);
 
   t->PrototypeTemplate()->SetAccessor(env->bytes_read_string(),
                                       GetBytesRead<Base>,
                                       nullptr,
                                       env->as_external(),
                                       v8::DEFAULT,
-                                      attributes);
+                                      attributes,
+                                      signature);
 
   env->SetProtoMethod(t, "readStart", JSMethod<Base, &StreamBase::ReadStart>);
   env->SetProtoMethod(t, "readStop", JSMethod<Base, &StreamBase::ReadStop>);
diff --git a/test/parallel/test-stream-base-prototype-accessors-enumerability.js b/test/parallel/test-stream-base-prototype-accessors-enumerability.js
new file mode 100644
index 00000000000000..f59aced197c515
--- /dev/null
+++ b/test/parallel/test-stream-base-prototype-accessors-enumerability.js
@@ -0,0 +1,19 @@
+'use strict';
+
+require('../common');
+
+// This tests that the prototype accessors added by StreamBase::AddMethods
+// are not enumerable. They could be enumerated when inspecting the prototype
+// with util.inspect or the inspector protocol.
+
+const assert = require('assert');
+
+// Or anything that calls StreamBase::AddMethods when setting up its prototype
+const TTY = process.binding('tty_wrap').TTY;
+
+{
+  assert.strictEqual(TTY.prototype.propertyIsEnumerable('bytesRead'), false);
+  assert.strictEqual(TTY.prototype.propertyIsEnumerable('fd'), false);
+  assert.strictEqual(
+    TTY.prototype.propertyIsEnumerable('_externalStream'), false);
+}
diff --git a/test/parallel/test-stream-base-prototype-accessors.js b/test/parallel/test-stream-base-prototype-accessors.js
new file mode 100644
index 00000000000000..f9e12582a098d8
--- /dev/null
+++ b/test/parallel/test-stream-base-prototype-accessors.js
@@ -0,0 +1,27 @@
+'use strict';
+
+require('../common');
+
+// This tests that the prototype accessors added by StreamBase::AddMethods
+// do not raise assersions when called with incompatible receivers.
+
+const assert = require('assert');
+
+// Or anything that calls StreamBase::AddMethods when setting up its prototype
+const TTY = process.binding('tty_wrap').TTY;
+
+// Should throw instead of raise assertions
+{
+  const msg = /TypeError: Method \w+ called on incompatible receiver/;
+  assert.throws(() => {
+    TTY.prototype.bytesRead;
+  }, msg);
+
+  assert.throws(() => {
+    TTY.prototype.fd;
+  }, msg);
+
+  assert.throws(() => {
+    TTY.prototype._externalStream;
+  }, msg);
+}