Skip to content

Commit 38fa2e6

Browse files
YongquYongqu
committed
[MERGE chakra-core#916] MSFT: 7363961 Array.prototype.concat doesn't check if a Proxy is revoked when the Proxy is "this"
Merge pull request chakra-core#916 from Yongqu:bugfixs There are places we directly access proxy's target & potentially handler after the proxy is revoked. At check when non-proxy code access the handler & target, throw if proxy was revoked already.
2 parents ee35e0f + aa9d385 commit 38fa2e6

6 files changed

Lines changed: 67 additions & 6 deletions

File tree

‎lib/Runtime/Debug/DiagObjectModel.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3622,6 +3622,10 @@ namespace Js
36223622
BOOL RecyclableProxyObjectWalker::Get(int i, ResolvedObject* pResolvedObject)
36233623
{
36243624
JavascriptProxy* proxy = JavascriptProxy::FromVar(instance);
3625+
if (proxy->GetTarget() == nullptr || proxy->GetHandler() == nullptr)
3626+
{
3627+
return FALSE;
3628+
}
36253629
if (i == 0)
36263630
{
36273631
pResolvedObject->name = _u("[target]");

‎lib/Runtime/Library/JavascriptObject.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -474,7 +474,7 @@ namespace Js
474474
break;
475475

476476
case TypeIds_Proxy:
477-
if (JavascriptOperators::IsArray(JavascriptProxy::FromVar(thisArg)->GetTarget()))
477+
if (JavascriptOperators::IsArray(thisArg))
478478
{
479479
if (!isES6ToStringTagEnabled || tag == nullptr || wcscmp(tag->UnsafeGetBuffer(), _u("Array")) == 0)
480480
{

‎lib/Runtime/Library/JavascriptProxy.cpp‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,25 @@ namespace Js
1212
return JavascriptOperators::GetTypeId(obj) == TypeIds_Proxy;
1313
}
1414

15+
RecyclableObject* JavascriptProxy::GetTarget()
16+
{
17+
if (target == nullptr)
18+
{
19+
JavascriptError::ThrowTypeError(GetScriptContext(), JSERR_ErrorOnRevokedProxy, _u(""));
20+
}
21+
return target;
22+
}
23+
24+
RecyclableObject* JavascriptProxy::GetHandler()
25+
{
26+
if (handler == nullptr)
27+
{
28+
JavascriptError::ThrowTypeError(GetScriptContext(), JSERR_ErrorOnRevokedProxy, _u(""));
29+
}
30+
return handler;
31+
}
32+
33+
1534
Var JavascriptProxy::NewInstance(RecyclableObject* function, CallInfo callInfo, ...)
1635
{
1736
PROBE_STACK(function->GetScriptContext(), Js::Constants::MinStackDefault);
@@ -59,7 +78,7 @@ namespace Js
5978
#endif
6079
if (JavascriptProxy::Is(target))
6180
{
62-
if (JavascriptProxy::FromVar(target)->GetTarget() == nullptr)
81+
if (JavascriptProxy::FromVar(target)->target == nullptr)
6382
{
6483
JavascriptError::ThrowTypeError(scriptContext, JSERR_InvalidProxyArgument, _u("target"));
6584
}
@@ -72,7 +91,7 @@ namespace Js
7291
handler = DynamicObject::FromVar(args[2]);
7392
if (JavascriptProxy::Is(handler))
7493
{
75-
if (JavascriptProxy::FromVar(handler)->GetHandler() == nullptr)
94+
if (JavascriptProxy::FromVar(handler)->handler == nullptr)
7695
{
7796
JavascriptError::ThrowTypeError(scriptContext, JSERR_InvalidProxyArgument, _u("handler"));
7897
}

‎lib/Runtime/Library/JavascriptProxy.h‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,13 @@ namespace Js
5252
JavascriptProxy(DynamicType * type, ScriptContext * scriptContext, RecyclableObject* target, RecyclableObject* handler);
5353
static BOOL Is(Var obj);
5454
static JavascriptProxy* FromVar(Var obj) { Assert(Is(obj)); return static_cast<JavascriptProxy*>(obj); }
55-
RecyclableObject* GetTarget() const { return target; }
56-
RecyclableObject* GetHandler() const { return handler; }
57-
55+
#ifndef IsJsDiag
56+
RecyclableObject* GetTarget();
57+
RecyclableObject* GetHandler();
58+
#else
59+
RecyclableObject* GetTarget() { return target; }
60+
RecyclableObject* GetHandler() { return handler; }
61+
#endif
5862
static Var NewInstance(RecyclableObject* function, CallInfo callInfo, ...);
5963
static Var EntryRevocable(RecyclableObject* function, CallInfo callInfo, ...);
6064
static Var EntryRevoke(RecyclableObject* function, CallInfo callInfo, ...);

‎test/es6/proxytest9.baseline‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,3 +118,6 @@ getTrap, property : foo123
118118
foo called
119119
43
120120
42
121+
expected :method is called on a revoked Proxy object
122+
expected :method get is called on a revoked Proxy object
123+
expected :method construct is called on a revoked Proxy object

‎test/es6/proxytest9.js‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,36 @@ function test29() {
532532
}
533533
}
534534

535+
function test30() {
536+
var o = Proxy.revocable([], {});
537+
o.revoke();
538+
try {
539+
Array.prototype.concat.call(o.proxy);
540+
} catch(e) {
541+
print('expected :' + e.message);
542+
}
543+
544+
try {
545+
Array.prototype.join.call(o.proxy, o.proxy);
546+
} catch(e) {
547+
print('expected :' + e.message);
548+
}
549+
550+
try {
551+
Object.prototype.toString.call(o.proxy);
552+
} catch(e) {
553+
print('expected :' + e.message);
554+
}
555+
556+
try {
557+
function foo() {return this;}
558+
var p = Proxy.revocable(foo, {});
559+
p.revoke();
560+
var pp = new p.proxy();
561+
} catch(e) {
562+
print('expected :' + e.message);
563+
}
564+
}
535565
test0();
536566
test1();
537567
test2();
@@ -562,3 +592,4 @@ test26();
562592
test27();
563593
test28();
564594
test29();
595+
test30();

0 commit comments

Comments
 (0)