Skip to content

Commit

Permalink
Check for deeply or infinitely nested Proxies
Browse files Browse the repository at this point in the history 
Summary:
A proxy whose target or handler is another proxy can result
in native stack exhaustion.  Insert checks for this case.

Reviewed By: avp

Differential Revision: D23662553

fbshipit-source-id: ec62795e8afdaafc83c301106c3229e8aa451586
  • Loading branch information
@mhorowitz @facebook-github-bot
mhorowitz authored and facebook-github-bot committed Sep 26, 2020
1 parent 8cb935c commit 4108bf9
Show file tree
Hide file tree
Showing 2 changed files with 112 additions and 4 deletions.
90 changes: 88 additions & 2 deletions lib/VM/JSProxy.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,15 @@ findTrap(Handle<JSObject> selfHandle, Runtime *runtime, Predefined::Str name) {
// 5. Let target be O.[[ProxyTarget]].
// 6. Let trap be ? GetMethod(handler, « name »).
Handle<JSObject> handler = runtime->makeHandle(handlerPtr);
// Calls to look up the trap are effectively recursion, and so
// require their own scope. They also need a
// ScopedNativeDepthTracker, as it's possible to use up arbitrary
// native stack depth with nested proxies.
GCScope gcScope(runtime);
ScopedNativeDepthTracker depthTracker(runtime);
if (LLVM_UNLIKELY(depthTracker.overflowed())) {
return runtime->raiseStackOverflow(Runtime::StackOverflowKind::NativeStack);
}
CallResult<PseudoHandle<>> trapVal =
JSObject::getNamed_RJS(handler, runtime, Predefined::getSymbolID(name));
if (trapVal == ExecutionStatus::EXCEPTION) {
Expand All @@ -57,7 +66,7 @@ findTrap(Handle<JSObject> selfHandle, Runtime *runtime, Predefined::Str name) {
runtime->makeHandle(std::move(*trapVal)),
" is not a Proxy trap function");
}
return runtime->makeHandle<Callable>(std::move(trapVal->get()));
return runtime->makeHandleInParentScope<Callable>(std::move(trapVal->get()));
}

} // namespace detail
Expand Down Expand Up @@ -280,8 +289,15 @@ CallResult<PseudoHandle<JSObject>> JSProxy::getPrototypeOf(
if (!*trapRes) {
// a. Return ? target.[[GetPrototypeOf]](P).
// All calls to the target in the no-trap case are effectively
// recursion, and so require their own scope.
// recursion, and so require their own scope. They also need a
// ScopedNativeDepthTracker, as it's possible to use up arbitrary
// native stack depth with nested proxies.
GCScope gcScope(runtime);
ScopedNativeDepthTracker depthTracker(runtime);
if (LLVM_UNLIKELY(depthTracker.overflowed())) {
return runtime->raiseStackOverflow(
Runtime::StackOverflowKind::NativeStack);
}
return JSObject::getPrototypeOf(target, runtime);
}
// 7. Let handlerProto be ? Call(trap, handler, « target »).
Expand Down Expand Up @@ -342,6 +358,11 @@ CallResult<bool> JSProxy::setPrototypeOf(
// 7. If trap is undefined, then
if (!*trapRes) {
GCScope gcScope(runtime);
ScopedNativeDepthTracker depthTracker(runtime);
if (LLVM_UNLIKELY(depthTracker.overflowed())) {
return runtime->raiseStackOverflow(
Runtime::StackOverflowKind::NativeStack);
}
// a. Return ? target.[[SetPrototypeOf]](V).
return JSObject::setParent(*target, runtime, *parent);
}
Expand Down Expand Up @@ -398,6 +419,11 @@ CallResult<bool> JSProxy::isExtensible(
// 6. If trap is undefined, then
if (!*trapRes) {
GCScope gcScope(runtime);
ScopedNativeDepthTracker depthTracker(runtime);
if (LLVM_UNLIKELY(depthTracker.overflowed())) {
return runtime->raiseStackOverflow(
Runtime::StackOverflowKind::NativeStack);
}
// a. Return ? target.[[IsExtensible]]().
return JSObject::isExtensible(target, runtime);
}
Expand Down Expand Up @@ -441,6 +467,11 @@ CallResult<bool> JSProxy::preventExtensions(
// 6. If trap is undefined, then
if (!*trapRes) {
GCScope gcScope(runtime);
ScopedNativeDepthTracker depthTracker(runtime);
if (LLVM_UNLIKELY(depthTracker.overflowed())) {
return runtime->raiseStackOverflow(
Runtime::StackOverflowKind::NativeStack);
}
// a. Return ? target.[[PreventExtensions]]().
// We pass in opFlags here. If getThrowOnError, then this will cause
// the underlying exception to bubble up. If !getThrowOnError, then
Expand Down Expand Up @@ -494,6 +525,11 @@ CallResult<bool> JSProxy::getOwnProperty(
// 7. If trap is undefined, then
if (!*trapRes) {
GCScope gcScope(runtime);
ScopedNativeDepthTracker depthTracker(runtime);
if (LLVM_UNLIKELY(depthTracker.overflowed())) {
return runtime->raiseStackOverflow(
Runtime::StackOverflowKind::NativeStack);
}
// a. Return ? target.[[GetOwnProperty]](P).
return valueOrAccessor
? JSObject::getOwnComputedDescriptor(
Expand Down Expand Up @@ -641,6 +677,11 @@ CallResult<bool> JSProxy::defineOwnProperty(
// 7. If trap is undefined, then
if (!*trapRes) {
GCScope gcScope(runtime);
ScopedNativeDepthTracker depthTracker(runtime);
if (LLVM_UNLIKELY(depthTracker.overflowed())) {
return runtime->raiseStackOverflow(
Runtime::StackOverflowKind::NativeStack);
}
// a. Return ? target.[[GetOwnProperty]](P).
return JSObject::defineOwnComputedPrimitive(
target, runtime, nameValHandle, dpFlags, valueOrAccessor, opFlags);
Expand Down Expand Up @@ -805,6 +846,11 @@ CallResult<bool> JSProxy::hasNamed(
// 7. If trap is undefined, then
if (!*trapRes) {
GCScope gcScope(runtime);
ScopedNativeDepthTracker depthTracker(runtime);
if (LLVM_UNLIKELY(depthTracker.overflowed())) {
return runtime->raiseStackOverflow(
Runtime::StackOverflowKind::NativeStack);
}
// a. Return ? target.[[HasProperty]](P, Receiver).
return JSObject::hasNamed(target, runtime, name);
}
Expand Down Expand Up @@ -832,6 +878,11 @@ CallResult<bool> JSProxy::hasComputed(
// 7. If trap is undefined, then
if (!*trapRes) {
GCScope gcScope(runtime);
ScopedNativeDepthTracker depthTracker(runtime);
if (LLVM_UNLIKELY(depthTracker.overflowed())) {
return runtime->raiseStackOverflow(
Runtime::StackOverflowKind::NativeStack);
}
// a. Return ? target.[[HasProperty]](P, Receiver).
return JSObject::hasComputed(target, runtime, nameValHandle);
}
Expand Down Expand Up @@ -921,6 +972,11 @@ CallResult<PseudoHandle<>> JSProxy::getNamed(
// 7. If trap is undefined, then
if (!*trapRes) {
GCScope gcScope(runtime);
ScopedNativeDepthTracker depthTracker(runtime);
if (LLVM_UNLIKELY(depthTracker.overflowed())) {
return runtime->raiseStackOverflow(
Runtime::StackOverflowKind::NativeStack);
}
// a. Return ? target.[[Get]](P, Receiver).
return JSObject::getNamedWithReceiver_RJS(target, runtime, name, receiver);
}
Expand Down Expand Up @@ -951,6 +1007,11 @@ CallResult<PseudoHandle<>> JSProxy::getComputed(
// 7. If trap is undefined, then
if (!*trapRes) {
GCScope gcScope(runtime);
ScopedNativeDepthTracker depthTracker(runtime);
if (LLVM_UNLIKELY(depthTracker.overflowed())) {
return runtime->raiseStackOverflow(
Runtime::StackOverflowKind::NativeStack);
}
// a. Return ? target.[[Get]](P, Receiver).
return JSObject::getComputedWithReceiver_RJS(
target, runtime, nameValHandle, receiver);
Expand Down Expand Up @@ -1050,6 +1111,11 @@ CallResult<bool> JSProxy::setNamed(
// 7. If trap is undefined, then
if (!*trapRes) {
GCScope gcScope(runtime);
ScopedNativeDepthTracker depthTracker(runtime);
if (LLVM_UNLIKELY(depthTracker.overflowed())) {
return runtime->raiseStackOverflow(
Runtime::StackOverflowKind::NativeStack);
}
// a. Return ? target.[[Set]](P, V, Receiver).
return JSObject::putNamedWithReceiver_RJS(
target, runtime, name, valueHandle, receiver);
Expand Down Expand Up @@ -1084,6 +1150,11 @@ CallResult<bool> JSProxy::setComputed(
// 7. If trap is undefined, then
if (!*trapRes) {
GCScope gcScope(runtime);
ScopedNativeDepthTracker depthTracker(runtime);
if (LLVM_UNLIKELY(depthTracker.overflowed())) {
return runtime->raiseStackOverflow(
Runtime::StackOverflowKind::NativeStack);
}
// a. Return ? target.[[Set]](P, V, Receiver).
return JSObject::putComputedWithReceiver_RJS(
target, runtime, nameValHandle, valueHandle, receiver);
Expand Down Expand Up @@ -1163,6 +1234,11 @@ CallResult<bool> JSProxy::deleteNamed(
// 7. If trap is undefined, then
if (!*trapRes) {
GCScope gcScope(runtime);
ScopedNativeDepthTracker depthTracker(runtime);
if (LLVM_UNLIKELY(depthTracker.overflowed())) {
return runtime->raiseStackOverflow(
Runtime::StackOverflowKind::NativeStack);
}
// a. Return ? target.[[Delete]](P, Receiver).
return JSObject::deleteNamed(target, runtime, name);
}
Expand Down Expand Up @@ -1190,6 +1266,11 @@ CallResult<bool> JSProxy::deleteComputed(
// 7. If trap is undefined, then
if (!*trapRes) {
GCScope gcScope(runtime);
ScopedNativeDepthTracker depthTracker(runtime);
if (LLVM_UNLIKELY(depthTracker.overflowed())) {
return runtime->raiseStackOverflow(
Runtime::StackOverflowKind::NativeStack);
}
// a. Return ? target.[[Delete]](P, Receiver).
return JSObject::deleteComputed(target, runtime, nameValHandle);
}
Expand Down Expand Up @@ -1297,6 +1378,11 @@ CallResult<PseudoHandle<JSArray>> JSProxy::ownPropertyKeys(
if (!*trapRes) {
// a. Return ? target.[[OwnPropertyKeys]]().
GCScope gcScope(runtime);
ScopedNativeDepthTracker depthTracker(runtime);
if (LLVM_UNLIKELY(depthTracker.overflowed())) {
return runtime->raiseStackOverflow(
Runtime::StackOverflowKind::NativeStack);
}
CallResult<Handle<JSArray>> targetRes =
// Include everything here, so that filterKeys has a chance to
// make observable trap calls.
Expand Down
26 changes: 24 additions & 2 deletions test/hermes/proxy.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2102,13 +2102,35 @@ arrayTests(
print('misc');
// CHECK-LABEL: misc

// Do a deep target recursion
// Do a deep target recursion. This needs to be within the smallest
// (ASAN) limit for tests to pass, but larger numbers will work in
// production builds.
var p = {a:1};
for (var i = 0; i < 64; ++i) {
for (var i = 0; i < 20; ++i) {
p = new Proxy(p, {});
}
assert.equal(p.a, 1);

// Do a really deep target recursion to test for stack overflow
var p = {a:1};
for (var i = 0; i < 10000; ++i) {
p = new Proxy({}, p);
}
checkThrows(RangeError)(_ => p.a);

// Do a really deep handler recursion to test for stack overflow
var p = {a:1};
for (var i = 0; i < 10000; ++i) {
p = new Proxy(p, {});
}
checkThrows(RangeError)(_ => p.a);

// Do an infinite recursion to test for stack overflow
var p1 = [];
var p2 = new Proxy(p1, {});
p1.__proto__ = p2;
checkThrows(RangeError)(_ => p1.a);

// Test HermesInternal
assert.equal(
typeof HermesInternal !== 'object' ||
Expand Down

0 comments on commit 4108bf9

Please sign in to comment.