Closed Bug 1795496 (CVE-2023-37456) Opened 2 years ago Closed 1 year ago

[iOS] Browser crashes when webkit.messageHandlers.sessionRestoreHelper.postMessage() is called with an empty body

Categories

(Firefox for iOS :: General, defect)

Unspecified
iOS
defect

Tracking

()

RESOLVED FIXED
Tracking Status
fxios 115 ---

People

(Reporter: chaykin.artem, Assigned: lmarceau)

References

()

Details

(Keywords: crash, csectype-dos, sec-low, Whiteboard: [reporter-external] [client-bounty-form] [verif?])

Attachments

(1 file)

Hi!

We at Brave have been reworking JS scripts and WebKit message handlers recently, so I also decided to look at your messageHandlers.
I found that in this line (https://github.com/mozilla-mobile/firefox-ios/blob/466e09a1f07ea080dad026d955e2ef0dd1aa30f0/Client/Frontend/TabContentsScripts/SessionRestoreHelper.swift#L26) you don't check if params["name"] exists, which leads to a full browser crash if a webpage calls postMessage() with an empty body:
webkit.messageHandlers.sessionRestoreHelper.postMessage({})

PoC:
https://stoletheminerals.github.io/SessionRestoreHelper.html

Tested on the latest Firefox version in AppStore and the one compiled from github sources.

I guess a proper fix would be to either add a guard check on params["name"] or move this message handler to the sandboxed context.

Thanks,
Artem

Flags: sec-bounty?
OS: Unspecified → iOS
Group: firefox-core-security → mobile-core-security
Component: Security → General
Product: Firefox → Firefox for iOS
Status: UNCONFIRMED → NEW
Ever confirmed: true

The severity field is not set for this bug.
:jeevans, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(jeevans)

Thank you for raising this. I opened a PR here to fix this with a guard statement. Progress is tracked with JIRA issue.

@dveditz even though this is marked as security issue, I think this is just a crash and doesn't need CVE and all, correct?
Thank you!

Flags: needinfo?(dveditz)

We don't usually issue CVEs for sec-low level bugs, but it's still a problem.

Why is web content even allowed to use the sessionRestoreHelper? shouldn't there be an appIDToken check or whatever you're using elsewhere? If a malicious page didn't cause a crash by passing nothing, what else could they cause that helper to do?

Are there other helpers written and accessible to content in the same way? Treating this as a security bug and not "just a crash" gives us the opportunity to investigate whether this was a one-time mistake, or if there are other places written under a similar misunderstanding that also need to be fixed.

So, the sessionRestoreHelper is called when the webpage sent a script message using WebKit message handler. The message passed to that swift code is of type WKScriptMessage.

:dveditz We do have some appIDToken checks (example here), but we don't have that check for all the scripts we have (and it's not present for the sessionRestoreHelper). I think this is another conversation not related to this ticket directly thought, so let's have this conversation elsewhere.

Severity: -- → S3
Flags: needinfo?(jeevans)

Verified as fixed on v115 (31516) with iPhone 13 Pro (15.7.1).
Please note that I was not able to reproduce the crash anymore on this version.
I also tried with older versions (v113, v113.1, v113.2) and when accessing https://stoletheminerals.github.io/SessionRestoreHelper.html Firefox crashed.

Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Flags: qe-verify+
Assignee: nobody → lmarceau
Group: mobile-core-security → core-security-release
Group: core-security-release
Flags: sec-bounty? → sec-bounty-
Alias: CVE-2023-37456
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: