Skip to content

Commit

Permalink
src: make StreamBase prototype accessors robust
Browse files Browse the repository at this point in the history
This PR makes the prototype accessors added by StreamBase::AddMethods
nonenumerable and checks the signatures in the accessors so they
throw instead of raising assertions when called with incompatible
receivers. They could be enumerated when inspecting the prototype
with util.inspect or the inspector protocol.

PR-URL: #16860
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
joyeecheung authored and MylesBorins committed Nov 17, 2017
1 parent 0d4f62c commit f7411b5
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 4 deletions.
15 changes: 11 additions & 4 deletions src/stream_base-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace node {

using v8::AccessorSignature;
using v8::External;
using v8::FunctionCallbackInfo;
using v8::FunctionTemplate;
Expand All @@ -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>);
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
27 changes: 27 additions & 0 deletions test/parallel/test-stream-base-prototype-accessors.js
Original file line number Diff line number Diff line change
@@ -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);
}

0 comments on commit f7411b5

Please sign in to comment.