1847372 - [@ java.lang.OutOfMemoryError: at java.util.Arrays.copyOf(Arrays.java) ] in HttpIconLoader due to excessive remote data sizes

Status: RESOLVED FIXED

(Fenix :: Browser Engine, defect, P2)

Description

[Rob Wu [:robwu]]

I received multiple reports from Dutch users of Firefox for Android (encountered on release, Beta, Nightly and Focus) that visiting a major Dutch banking website (https://www.ing.nl) results in an immediate crash of the browser. The culprit appears to be the presence of the following snippet, in ing.nl/particulier (web archive copy), which references an image of 123 693 356 bytes:

<meta property="og:image" content="https://assets.ing.com/m/14ba5185d4fc6d64/original/WK-vrouwenvoetbal-ongelijke-doelen.tif"/>

After examining the stack trace (below), I am not surprised:

• Response.toIconLoaderResult tries to read the full bytes of an arbitrary web response (source of HttpIconLoader.kt).
• The URLs to fetch are scraped from the web page (source of browser-icons/icons.js component).

This logic is very questionable. The main Firefox UI process should not fetch and store images of an arbitrary size (file size and image canvas size). Besides the obvious OOM issue, there is also the concern of wasting the bandwidth of mobile users, especially on expensive phone plans. I recommend to enforce a maximum number of permitted bytes, and if feasible to further minimize memory usage by compressing the image thumbnails. Gecko already has all of that functionality, so it may make sense to rely on Gecko rather than re-implementing this in A-C.

Example of stack trace: https://sentry.io/organizations/mozilla/issues/?project=6375561&query=b350379fa6704bf2ac0f9ad8b8ba6f8d

116.0 (Build #2015964771), f24b2b787e+
GV: 116.0-20230727152340
AS: 116.0
Pixel 2, Android 11, RP1A.201005.004.A1

java.lang.OutOfMemoryError: Failed to allocate a 134217744 byte allocation with 25112576 free bytes and 106MB until OOM, target footprint 115021616, growth limit 201326592
	at java.util.Arrays.copyOf(Arrays.java:3161)
	at java.io.ByteArrayOutputStream.grow(ByteArrayOutputStream.java:118)
	at java.io.ByteArrayOutputStream.ensureCapacity(ByteArrayOutputStream.java:93)
	at java.io.ByteArrayOutputStream.write(ByteArrayOutputStream.java:153)
	at kotlin.io.ByteStreamsKt.copyTo(IOStreams.kt:17)
	at kotlin.io.ByteStreamsKt.readBytes(IOStreams.kt:21)
	at mozilla.components.browser.icons.loader.HttpIconLoaderKt$toIconLoaderResult$1.invoke(HttpIconLoader.kt:10)
	at mozilla.components.concept.fetch.Response$Body.useStream(Response.kt:8)
	at mozilla.components.browser.icons.loader.HttpIconLoader.internalLoad(HttpIconLoader.kt:84)
	at mozilla.components.browser.icons.loader.HttpIconLoader.load(HttpIconLoader.kt:27)
	at mozilla.components.browser.icons.BrowserIconsKt.access$load(BrowserIcons.kt:61)
	at mozilla.components.browser.icons.BrowserIcons$loadIconInternalAsync$1.invokeSuspend(BrowserIcons.kt:113)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:9)
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:112)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
	at java.lang.Thread.run(Thread.java:923)
	Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException: [StandaloneCoroutine{Cancelling}@e17661, Dispatchers.IO]

Comment 1

[Rob Wu [:robwu]]

Linking bug 1770989 for visibility. If the implementation is fully refactored to fully rely on Gecko, then this bug could be fixed.

Before closing this bug, we should have at least a test case that links an excessive file, and verify that the engine does NOT attempt to load all of it.

Comment 2

[Chris Peterson [:cpeterson]]

QA, can you reproduce in a recent version?

I see recent OOM crash reports involving mozilla.components.browser.icons.loader.HttpIconLoaderKt$toIconLoaderResult$1.invoke, but the most recent version is 111:

https://crash-stats.mozilla.org/search/?java_stack_trace=~mozilla.components.browser.icons.loader.HttpIconLoaderKt%24toIconLoaderResult&product=Fenix&product=Focus&process_type=parent&date=%3E%3D2023-02-05T21%3A04%3A00.000Z&date=%3C2023-08-05T21%3A04%3A00.000Z&_facets=product&_facets=version&_facets=signature&_facets=major_version&_sort=-date&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#facet-major_version

Comment 3

[Rob Wu [:robwu]]

FYI if you cannot reproduce with the original test case, you can try to create a more extreme test case as follows:

1. Find or create a large file and make it available online. It might not even need to be an actual image.
2. Create a HTML file with the following content: <meta property="og:image" content="http(s) URL here"/> (with the "http(s) URL here" part replaced with the actual URL from step 1). Host this file online as well, preferably on a new domain that you haven't visited before.
3. Open the HTML file from step 3 (in private browsing mode?).

Result: crash.

Comment 4

[Rob Wu [:robwu]]

(In reply to Chris Peterson [:cpeterson] from comment #2)

QA, can you reproduce in a recent version?

By the way, as stated in the reports, the crash affects all products, latest versions included. I asked one of the affected people to install Firefox Beta from the Play Store, and they did, and were able to reproduce immediately,

I see recent OOM crash reports involving mozilla.components.browser.icons.loader.HttpIconLoaderKt$toIconLoaderResult$1.invoke, but the most recent version is 111:

Are Sentry reports aggregated and included in Crash stats? When I tried to reproduce on Beta, I hit the NPE in the crash reporter from bug 1838389.
Judging by that bug, the issue should be fixed on Nightly, so I updated Fenix Nightly to the latest, and repeated the STR of this bug. The crash report can submit successfully this time, but the report has "[@ EMPTY: no frame data available ]". Despite the empty signature, the Java exception lists the stack trace I mentioned in the initial report (I have protected data access and am therefore able to see that information; without authentication or access this info is not visible).

bp-3f3611e7-97e4-4db1-8061-0541b0230806

Comment 5

[Chris Peterson [:cpeterson]]

Thanks for confirming. In that case, no need for QA to reproduce.

Comment 6

[Cathy Lu [:calu]]

Hey Rob, thanks for bringing this bug up and providing the details. Is this bug blocked on refactoring the implementation so GeckoView can use it (bugs https://bugzilla.mozilla.org/show_bug.cgi?id=1770989 and https://bugzilla.mozilla.org/show_bug.cgi?id=1770990) or is it serious enough that we should consider implementing it in AC and any idea what that would look like?

Comment 7

[Rob Wu [:robwu]]

This bug is not blocked on the refactoring. Comment 1 mentions that the refactor could reduce the impact of the bug because Gecko would then take care of limiting the size of the image.

Independently of the Gecko-side refactoring, this bug can be resolved as described in the initial report. The fact that arbitrary websites can crash the whole browser process is a serious issue.

If the refactor is already being worked on, it would be ok to await that (and use this bug for a unit test). If it hasn't been scheduled yet, I suggest to fix this bug without awaiting a refactoring.

Comment 8

[Cathy Lu [:calu]]

Hey QA,

I'm unable to reproduce this OutOfMemory crash for the Dutch banking website (https://www.ing.nl and the archive in the first comment). I also tried hosting a large image mentioning in the STR in (comment 3)[https://bugzilla.mozilla.org/show_bug.cgi?id=1847372#c3] but it's not recognizing it as an icon and skipping past the code that causes the crash.

Can you find other sites that cause this crash?

Comment 9

[Rob Wu [:robwu]]

I can still reproduce, with the following test case (comparable to the test case from comment 3).

When I open a HTML file with the following content (referencing a 142 MB file), Firefox crashes after a few seconds:

<link rel="image_src" href="https://ftp.mozilla.org/pub/firefox/releases/125.0/mac/en-US/Firefox 125.0.pkg">

bp-cdd30995-dc23-4453-89d4-8d1080240415

If you cannot reproduce, try a larger file.

From about:crashes, I copied the error associated with the above test case:

72dbdaf6-ece3-41ff-9c30-36c7c6e11d7b
java.lang.OutOfMemoryError: Failed to allocate a 268435472 byte allocation with 100663296 free bytes and 103MB until OOM, target footprint 260350368, growth limit 268435456
 * New Sentry Instance: https://sentry.io/organizations/mozilla/issues/?project=6295551&query=76efa02436f64c60a3a705138606ff3c
 * Socorro: https://crash-stats.mozilla.org/report/index/bp-cdd30995-dc23-4453-89d4-8d1080240415
----
java.lang.OutOfMemoryError: Failed to allocate a 268435472 byte allocation with 100663296 free bytes and 103MB until OOM, target footprint 260350368, growth limit 268435456
        at java.util.Arrays.copyOf(Arrays.java:3578)
        at java.io.ByteArrayOutputStream.grow(ByteArrayOutputStream.java:120)
        at java.io.ByteArrayOutputStream.ensureCapacity(ByteArrayOutputStream.java:95)
        at java.io.ByteArrayOutputStream.write(ByteArrayOutputStream.java:156)
        at kotlin.io.ByteStreamsKt.copyTo(IOStreams.kt:17)
        at kotlin.io.ByteStreamsKt.copyTo$default(IOStreams.kt:3)
        at kotlin.io.ByteStreamsKt.readBytes(IOStreams.kt:21)
        at mozilla.components.browser.icons.loader.HttpIconLoaderKt$toIconLoaderResult$1.invoke(HttpIconLoader.kt:10)
        at mozilla.components.concept.fetch.Response$Body.useStream(Response.kt:8)
        at mozilla.components.browser.icons.loader.HttpIconLoader.internalLoad(HttpIconLoader.kt:85)
        at mozilla.components.browser.icons.loader.HttpIconLoader.load(HttpIconLoader.kt:25)
        at mozilla.components.browser.icons.BrowserIconsKt.access$load(BrowserIcons.kt:79)
        at mozilla.components.browser.icons.BrowserIcons$loadIconInternalAsync$1.invokeSuspend(BrowserIcons.kt:93)
        at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:9)
        at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:111)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:644)
        at java.lang.Thread.run(Thread.java:1012)
        Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException: [StandaloneCoroutine{Cancelling}@5c4ce8d, Dispatchers.IO]

Comment 10

[Cathy Lu [:calu]]

Attachment: Bug 1847372 - Set a maximum file size and OutOfMemory Exception for loading large http icons

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eb485dfad085
Set a maximum file size and OutOfMemory Exception for loading large http icons r=android-reviewers,rsainani

Comment 12

[Sandor Molnar[:smolnar]]

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

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:calu, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox126 to wontfix.

For more information, please visit BugBot documentation.

Comment 14

[Donal Meehan [:dmeehan]]

This won't be included in 126.0 RC1. However, we could consider including it in a dot release depending on the risk.
There's already a pending needinfo for :calu., if it's safe to a release uplift request on this for consideration.

Comment 15

[Cathy Lu [:calu]]

Updated status-firefox126 to wontfix. No need for release uplift, as there's some risk since there is a new maximum size for icons.