-
Notifications
You must be signed in to change notification settings - Fork 868
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[iOS] - Fix securing message handlers on iOS-18 #26014
Conversation
Nice catch @Brandon-T |
delete window['webkit']; | ||
|
||
// WebKit no longer restores the handler immediately! So we poll for when that happens and resolve the promise accordingly. | ||
const timeout = 5000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few concerns I have.
- Does it wait 5 seconds before trying to
postMessage
? Isn't it a lot for some use cases? - Also a web page can race during this time out and modify
window.webkit
once it is restored (or before it is restored, I'm not sure what happens in this case)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Fixed.
- Fixed.
I tested with:
let interval = setInterval(function() {
console.log("Replace window.webkit with hacked version");
}, 0);
setTimeout(function() {
let startTime = Date.now();
while(true) {
console.log("Polling for window.webkit legit version");
if (Date.now() - startTime >= 5000) {
clearInterval(interval);
break;
}
}
}, 0);
To see if the loop will block
the setInterval
. So even if a page tries to replace the window.webkit
inside of setInterval
or something similar, the while loop is synchronous and it blocks ALL other operations (async included).
Only once the while loop completes, everything else executes :) so I changed the code to use a while loop with a timeout. I checked performance and there's no issue as webkit replaces it internally asynchronously, in the next execution frame, while the loop blocks the page from executing anything else.
…ange to a synchronous polling while-loop.
Released in v1.73.22 |
Resolves brave/brave-browser#41653
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
wikinpm run presubmit
wiki,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan: