Implement the IDL security check#28583
Conversation
|
Heads up! This PR modifies the following files:
|
5abae38 to
6388d17
Compare
|
☔ The latest upstream changes (presumably #29384) made this pull request unmergeable. Please resolve the merge conflicts. |
|
This is great, and I love the CallPolicy additions. I'm going to rebase these changes and get them merged. |
6388d17 to
3bbd757
Compare
…ion parameters This trait's associated constant `INFO` provides fields that control various details of an IDL operation, such as whether a `[LegacyLenientThis]` attribute is specified on the corresponding interface member. The `generic_(method|getter|setter)` functions are now generic over `Policy: CallPolicy`, making the `generic_lenient_*` methods unnecessary. Currently, they pass `Policy::INFO` to the inner `generic_method` function as a runtime parameter, but we can change it later if we find it beneficial to make generic the inner function. Signed-off-by: Josh Matthews <[email protected]>
<https://heycam.github.io/webidl/#es-operations> <https://html.spec.whatwg.org/multipage/#integration-with-idl> This security check is an important (but not only) mechanism to restrict cross-origin DOM accesses. Signed-off-by: Josh Matthews <[email protected]>
…avior <https://heycam.github.io/webidl/#es-operations> <https://html.spec.whatwg.org/multipage/#integration-with-idl> Not implementing the specification's behavior for the following reasons: - Neither Gecko nor WebKit implements this behavior. - This would require an elaborate mechanism to have access to the target object's `CrossOriginProperties`. Signed-off-by: Josh Matthews <[email protected]>
Signed-off-by: Josh Matthews <[email protected]>
3bbd757 to
b42ebb0
Compare
|
After a detour to fix javascript: URL evaluation which was causing lots of tests to fail intermittently, this is now ready to merge! |
Signed-off-by: Josh Matthews <[email protected]>
Signed-off-by: Josh Matthews <[email protected]>
|
hi! i’m working on the monthly update. i did a bunch of testing to understand exactly what this patch fixes and what it doesn’t fix, but i’m still not 100% sure. it seems like before this patch, cross-site DOM accesses were already correctly rejected by servo, but same-site cross-origin DOM accesses were incorrectly allowed by servo. and after this patch, we fix one kind of incorrect access, specifically those that involve binding a Window or Location method in this document with a <!--
1. save this file as test.html
2. serve the directory with both `python -m http.server 8000` and `8001`
3. go to <http://localhost:8000/test.html>
-->
<!-- case 1: same-origin -->
<!-- <iframe id=i src="http://localhost:8000/"></iframe> -->
<!-- case 2: same-site cross-origin -->
<iframe id=i src="http://localhost:8001/"></iframe>
<!-- case 3: cross-site -->
<!-- <iframe id=i src="https://servo.org/"></iframe> -->
<script>
setTimeout(()=>{
try {
// for case 2...
// servo 0.1.0: returns `"#document"`
// servo 0.2.0: returns `"#document"`
// firefox 150: throws DOMException
alert(i.contentWindow.document.nodeName);
} catch (e) {
console.log(e);
}
try {
// for case 2...
// servo 0.1.0: returns `undefined`
// servo 0.2.0: returns `undefined`
// firefox 150: throws DOMException
alert(i.contentWindow.stop());
} catch (e) {
console.log(e);
}
try {
// for case 2...
// servo 0.1.0: returns `undefined`
// servo 0.2.0: throws SecurityError
// firefox 150: throws DOMException
const d = Object.getOwnPropertyDescriptor(window,"stop");
alert(d.value.bind(i.contentWindow)());
} catch (e) {
console.log(e);
}
},250)
</script> |
|
That is correct. This only affects the exotic WebIDL interfaces, Window and Location. #44649 is another big missing piece here. |
https://heycam.github.io/webidl/#es-operations
https://html.spec.whatwg.org/multipage/#integration-with-idl
This PR implements the IDL security check, which is an important (but not only) mechanism to restrict cross-origin DOM accesses.
This implementation follows WebKit's behavior and not the specification for the following reasons:
CrossOriginProperties../mach build -ddoes not report any errors./mach test-tidydoes not report any errors