-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Cherry-pick 272448.103@safari-7618-branch (e3a7580). https://bugs.web…
…kit.org/show_bug.cgi?id=267425 [JSC] get_by_id_with_this + ProxyObject can leak JSScope objects https://bugs.webkit.org/show_bug.cgi?id=267425 <rdar://120777816> Reviewed by Yusuke Suzuki and Justin Michaud. According to the spec [1], `var base = { foo }; with (base) foo();` should be called with `this` value of `base`, which is why FunctionCallResolveNode moves resolved scope to thisRegister(). That is arguably a bad design, and there is an effort [2] to abolish using JSScope as `this` value. When `this` value is accessed by JS code, it's being sanitized via ToThis (JSScope replaced with `undefined`), yet not in case of `super.property` access calling into ProxyObject `get` trap, which passes raw `this` value as receiver parameter, leaking JSScope to be exploited. For performance reasons, we can't call toThis() whenever `get_by_id_with_this` is used, so this change introduces @tothis() intrinsic specifically for ProxyObject IC helpers, tweaks DFG to respect `m_srcDst`, and also fixes baseline code. Inlineability of ProxyObject IC helpers was verified to remain unaffected (`performProxyObjectGet` is smaller then 120 while other helpers were already exceeding inline size limit). [1]: https://tc39.es/ecma262/#sec-evaluatecall (step 1.b.iii) [2]: https://bugs.webkit.org/show_bug.cgi?id=225397 * JSTests/stress/regress-120777816.js: Added. * Source/JavaScriptCore/builtins/ProxyHelpers.js: (linkTimeConstant.performProxyObjectGet): (linkTimeConstant.performProxyObjectGetByVal): (linkTimeConstant.performProxyObjectSetSloppy): (linkTimeConstant.performProxyObjectSetStrict): * Source/JavaScriptCore/bytecode/BytecodeIntrinsicRegistry.h: * Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp: (JSC::BytecodeGenerator::emitToThis): * Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h: (JSC::BytecodeGenerator::emitToThis): * Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp: (JSC::BytecodeIntrinsicNode::emit_intrinsic_toThis): * Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp: (JSC::DFG::ByteCodeParser::parseBlock): (JSC::DFG::ByteCodeParser::getThis): Deleted. (JSC::DFG::ByteCodeParser::setThis): Deleted. * Source/JavaScriptCore/runtime/ProxyObject.cpp: (JSC::performProxyGet): (JSC::ProxyObject::performPut): Canonical link: https://commits.webkit.org/272448.103@safari-7618-branch Canonical link: https://commits.webkit.org/274313.65@webkitglib/2.44
- Loading branch information
Showing
8 changed files
with
119 additions
and
22 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
function assert(x) { | ||
if (!x) | ||
throw new Error("Bad assertion!"); | ||
} | ||
|
||
(function() { | ||
function tryToLeakThisViaGetById() { | ||
class Leaker { | ||
leak() { | ||
return super.foo; | ||
} | ||
} | ||
|
||
Leaker.prototype.__proto__ = new Proxy({}, { | ||
get(target, propertyName, receiver) { | ||
return receiver; | ||
} | ||
}); | ||
|
||
const foo = 42; | ||
const {leak} = Leaker.prototype; | ||
|
||
return (() => leak())(); | ||
} | ||
|
||
function tryToLeakThisViaGetByVal() { | ||
class Leaker { | ||
leak() { | ||
return super[Math.random() < 0.5 ? "foo" : "bar"]; | ||
} | ||
} | ||
|
||
Leaker.prototype.__proto__ = new Proxy({}, { | ||
get(target, propertyName, receiver) { | ||
return receiver; | ||
} | ||
}); | ||
|
||
const foo = 42; | ||
const bar = 84; | ||
const {leak} = Leaker.prototype; | ||
|
||
return (() => leak())(); | ||
} | ||
|
||
function tryToLeakThisViaSetById() { | ||
let receiver; | ||
class Leaker { | ||
leak() { | ||
super.foo = {}; | ||
return receiver; | ||
} | ||
} | ||
Leaker.prototype.__proto__ = new Proxy({}, { | ||
set(target, propertyName, value, __receiver) { | ||
receiver = __receiver; | ||
return true; | ||
} | ||
}); | ||
|
||
const foo = 42; | ||
const {leak} = Leaker.prototype; | ||
|
||
return (() => leak())(); | ||
} | ||
|
||
function tryToLeakThisViaSetByVal() { | ||
let receiver; | ||
class Leaker { | ||
leak() { | ||
super[Math.random() < 0.5 ? "foo" : "bar"] = {}; | ||
return receiver; | ||
} | ||
} | ||
|
||
Leaker.prototype.__proto__ = new Proxy({}, { | ||
set(target, propertyName, value, __receiver) { | ||
receiver = __receiver; | ||
return true; | ||
} | ||
}); | ||
|
||
const foo = 42; | ||
const bar = 84; | ||
const {leak} = Leaker.prototype; | ||
|
||
return (() => leak())(); | ||
} | ||
|
||
for (var i = 0; i < 1e5; i++) { | ||
assert(tryToLeakThisViaGetById() === undefined); | ||
assert(tryToLeakThisViaGetByVal() === undefined); | ||
assert(tryToLeakThisViaSetById() === undefined); | ||
assert(tryToLeakThisViaSetByVal() === undefined); | ||
} | ||
})(); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters