Closed Bug 1626687 Opened 3 months ago Closed 3 months ago

Big data urls will crash client applications.

Categories

(GeckoView :: General, defect, P1)

Unspecified
All
defect

Tracking

(firefox77 fixed)

RESOLVED FIXED
mozilla77
Tracking Status
firefox77 --- fixed

People

(Reporter: petru, Assigned: owlish)

Details

(Whiteboard: [geckoview:m77])

Attachments

(1 file)

A recent report on Fenix indicated an issue in GeckoView: there is no cap on the size of a data url.
A too big one can cause all kinds of problems in client applications.

A scenario seen in Fenix for this is described in https://github.com/mozilla-mobile/fenix/issues/7341

STRs:

  1. Open Fenix.
  2. Go to: https://www.color-blindness.com/coblis-color-blindness-simulator/ .
  3. Choose any of the filters to apply, like "Green-Week/Deuteranomaly".
  4. Tap on the "Browse" button and choose the image you want to change
    (pick one having a few MBs in size)
    If the "Browse" button is not showing it might overflow the screen. To see it:
    • put the device in landscape mode or
    • tap on the "3 dots" from bottom right corner -> enable "Desktop Site" option.
  5. Wait for the image to be processed and the website to show a "Open simulated image in new window"

Result:

  • Long pressing on the above link will throw an "OutOfMemoryException"
  • Pressing on the link will open a new tab but crash with a "TransactionTooLargeException"
    Browser session would also get corrupted and so a user would have to clear data / reinstall.

This issue affects Fennec also (though it is more resilient than Fenix and needs a bigger data url to crash),
Chrome and Opera also (which will also be unable to display the result and may get into weird states but do not crash)

I think this issue should be resolved in GeckoView by just capping the url.

Please note that on Android we do have a hard limit of 1MB for all Binder transactions concurrently happening in the process.
Trying to share a big url / pass it to another Android app/service through Intents would also cause a crash.
I think this is another important scenario that would require capping the url in GeckoView.

Whiteboard: [geckoview:m77]
Priority: -- → P1
Assignee: nobody → bugzeeeeee

:snorp suggested it might make more sense to do this in Necko. I tested this a bit on a desktop FF, and, while it's definitely somewhat more resilient than Fenix, it still crashes at a certain point - I had to use a 8.8M pic to crash my FF tab. I tried to repro this on Chrome browser - it just opens about:blank in response to any of these links.

So I talked to :valentin about this, asking if we could truncate or do something similar to these data urls in Necko without breaking any FF functionality. His reply:

yes, that’s certainly a possibility. if you have a reproducible test case, it would be better to attempt to use fallible allocation instead, where possible. that might improve the situation a little.

(In reply to [:owlish] from comment #1)

:snorp suggested it might make more sense to do this in Necko. I tested this a bit on a desktop FF, and, while it's definitely somewhat more resilient than Fenix, it still crashes at a certain point - I had to use a 8.8M pic to crash my FF tab. I tried to repro this on Chrome browser - it just opens about:blank in response to any of these links.

So I talked to :valentin about this, asking if we could truncate or do something similar to these data urls in Necko without breaking any FF functionality. His reply:

yes, that’s certainly a possibility. if you have a reproducible test case, it would be better to attempt to use fallible allocation instead, where possible. that might improve the situation a little.

A fallible allocation will help desktop Firefox here, but it won't help GeckoView, because our problem lies with allocating in the Dalvik heap. Apparently Chrome limits data URIs to 2MB. Maybe that would be a reasonable choice for us as well?

After some more discussion with :valentin, the best course of action at this point would be to make sure Necko doesn't pass huge things into Android. I will be filing another follow-up bug report for the desktop crash.

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (he/him) from comment #2)

(In reply to [:owlish] from comment #1)

A fallible allocation will help desktop Firefox here, but it won't help GeckoView, because our problem lies with allocating in the Dalvik heap. Apparently Chrome limits data URIs to 2MB. Maybe that would be a reasonable choice for us as well?

Thank you, I'll look into that!

Pushed by istorozhko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a5106c6cd037
Do not handle data URIs larger than 2M on mobile r=necko-reviewers,geckoview-reviewers,valentin,snorp
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla77
You need to log in before you can comment on or make changes to this bug.