Skip to content

Commit 9d3835c

Browse files
committed
[MERGE #5529 @VSadov] Avoid deep recursion in GetTypeOfString when dealing with proxies
Merge pull request #5529 from VSadov:fix18260402 It is unusual, but possible that proxies contain proxies and could be deeply nested. Recursion over such objects causes stack overflow and should be avoided. With this fix we will unwrap proxies to the inermost proxy without recursing. Fixes: OS:18260402 Found by OSS-Fuzz
2 parents 97caf12 + 001f924 commit 9d3835c

File tree

4 files changed

+47
-4
lines changed

4 files changed

+47
-4
lines changed

lib/Runtime/Library/JavascriptProxy.cpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1621,16 +1621,18 @@ namespace Js
16211621

16221622
BOOL JavascriptProxy::GetDiagTypeString(StringBuilder<ArenaAllocator>* stringBuilder, ScriptContext* requestContext)
16231623
{
1624+
const JavascriptProxy* proxy = UnwrapNestedProxies(this);
1625+
16241626
//RecyclableObject* targetObj;
1625-
if (this->handler == nullptr)
1627+
if (proxy->handler == nullptr)
16261628
{
16271629
ThreadContext* threadContext = GetScriptContext()->GetThreadContext();
16281630
// the proxy has been revoked; TypeError.
16291631
if (!threadContext->RecordImplicitException())
16301632
return FALSE;
16311633
JavascriptError::ThrowTypeError(GetScriptContext(), JSERR_ErrorOnRevokedProxy, _u("getTypeString"));
16321634
}
1633-
return target->GetDiagTypeString(stringBuilder, requestContext);
1635+
return proxy->target->GetDiagTypeString(stringBuilder, requestContext);
16341636
}
16351637

16361638
RecyclableObject* JavascriptProxy::ToObject(ScriptContext * requestContext)
@@ -1649,7 +1651,9 @@ namespace Js
16491651

16501652
Var JavascriptProxy::GetTypeOfString(ScriptContext* requestContext)
16511653
{
1652-
if (this->handler == nullptr)
1654+
const JavascriptProxy* proxy = UnwrapNestedProxies(this);
1655+
1656+
if (proxy->handler == nullptr)
16531657
{
16541658
// even if handler is nullptr, return typeof as "object"
16551659
return requestContext->GetLibrary()->GetObjectTypeDisplayString();
@@ -1662,7 +1666,7 @@ namespace Js
16621666
else
16631667
{
16641668
// handle nested cases recursively
1665-
return this->target->GetTypeOfString(requestContext);
1669+
return proxy->target->GetTypeOfString(requestContext);
16661670
}
16671671
}
16681672

lib/Runtime/Library/JavascriptProxy.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,26 @@ namespace Js
5454
static BOOL Is(_In_ RecyclableObject* obj);
5555
static JavascriptProxy* FromVar(Var obj) { AssertOrFailFast(Is(obj)); return static_cast<JavascriptProxy*>(obj); }
5656
static JavascriptProxy* UnsafeFromVar(Var obj) { Assert(Is(obj)); return static_cast<JavascriptProxy*>(obj); }
57+
58+
// before recursively calling something on 'target' use this helper in case there is nesting of proxies.
59+
// the proxies could be deep nested and cause SO when processed recursively.
60+
static const JavascriptProxy* UnwrapNestedProxies(const JavascriptProxy* proxy)
61+
{
62+
// continue while we have a proxy that is not revoked
63+
while (proxy->handler != nullptr)
64+
{
65+
JavascriptProxy* nestedProxy = JavascriptOperators::TryFromVar<JavascriptProxy>(proxy->target);
66+
if (nestedProxy == nullptr)
67+
{
68+
break;
69+
}
70+
71+
proxy = nestedProxy;
72+
}
73+
74+
return proxy;
75+
}
76+
5777
#ifndef IsJsDiag
5878
RecyclableObject* GetTarget();
5979
RecyclableObject* GetHandler();

test/es6/ProxyInProxy.baseline

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,3 +112,5 @@ function
112112
*** proxied function and Function.prototype.toString.call
113113
function a() { }
114114
function a() { }
115+
*** deeply nested proxy and typeof
116+
pass

test/es6/ProxyInProxy.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,10 +222,27 @@ function test7() {
222222
// "function a() { }"
223223
}
224224

225+
function test8() {
226+
print("*** deeply nested proxy and typeof");
227+
228+
var nestedProxy = Proxy.revocable([], {}).proxy;
229+
for (let i = 0; i < 1e5; i++) {
230+
nestedProxy = new Proxy(nestedProxy, {});
231+
}
232+
233+
(function () {
234+
if (nestedProxy != null && typeof nestedProxy == "object")
235+
{
236+
console.log("pass");
237+
}
238+
})();
239+
}
240+
225241
test1();
226242
test2();
227243
test3();
228244
test4();
229245
test5();
230246
test6();
231247
test7();
248+
test8();

0 commit comments

Comments
 (0)