1626687 - Big data urls will crash client applications.

Status: RESOLVED FIXED

(GeckoView :: General, defect, P1)

Description

[Petru-Mugurel Lingurar [:petru]]

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.

Comment 1

[[:owlish] 🦉 PST]

: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.

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

(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?

Comment 3

[[:owlish] 🦉 PST]

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.

Comment 4

[[:owlish] 🦉 PST]

(In reply to James Willcox (:snorp) ([e-mail]) (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!

Comment 5

[[:owlish] 🦉 PST]

Attachment: Bug 1626687 - Do not handle data URIs larger than 2M on mobile

Comment 6

[Pulsebot]

Pushed by [e-mail]:
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

Comment 7

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/a5106c6cd037