1048387 - Crash at android.os.TransactionTooLargeException: at android.os.BinderProxy.transact(Native Method) at org.mozilla.gecko.PromptService.show(PromptService.java)

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Rafay Baloch]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/36.0.1985.125 Safari/537.36

Steps to reproduce:

Hi, 

The following code crashes my firefox for android:

<script>
frame = "<iframe src=\"test:";
for (i = 0; i < 100000; i++) {
frame = frame + "0000000000000000000000000000000000"; 
}
frame = frame + "\" width=125 height=125></iframe>";
document.write(frame); 
</script

You can see it here in action:
http://drhack.net/Content/QmobileDos.html


Actual results:

Firefox for android crashes resulting in a Denial Of service. 



Expected results:

Firefox should have handled the code and should not had crashes.

Comment 1

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

I was able to reproduce this and got the following crash
https://crash-stats.mozilla.com/report/index/b571359e-7763-41f9-b38d-83b2f2140804

this is not a security bug and we already have an open bug on the issue.

Comment 2

[Kevin Brosnan [Ex-Mozilla]]

The bug you duped to talks about canvas crashing. This is not canvas. I would prefer to keep them separate. 

The script builds a very long iframe src, 3,400,005 characters if I calculated correctly.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

GeckoAppShell.getHandlersForURL (and other callers of GeckoAppShell.getHandlersForIntent) should either sanitize incoming data (which might be tricky) or try/catch the call to getHandlersForIntent.

The test case is an extreme edge case, but shows Android has it's limits.

Comment 4

[Daniel Veditz [:dveditz]]

In this case we seem to be crashing in Android itself, but with a somewhat shorter URL this could get passed through the intent to some other app and then do bad stuff in there. Do we always ask the user before launching some other app? We don't want our browser to be a vector for exploits in other popular apps. This has been a real problem in the windows world, for example, so we ask before launching a helper app by default.

Comment 5

[Matt Wobensmith [:mwobensmith][:matt:]]

Mark, do you think you could find someone to debug this further? We'd like to know if it's just a DoS or if it's worse than that. Thank you.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Daniel Veditz [:dveditz] from comment #4)
> In this case we seem to be crashing in Android itself, but with a somewhat
> shorter URL this could get passed through the intent to some other app and
> then do bad stuff in there. Do we always ask the user before launching some
> other app? We don't want our browser to be a vector for exploits in other
> popular apps. This has been a real problem in the windows world, for
> example, so we ask before launching a helper app by default.

Yes, we wait for a user action before sending intents out to Android. For some situations, we prompt, for others we wait for the user to tap a UI widget in the URLBar (helper app).

The crash in this bug is about getting a URL and asking Android if any Native apps are registered to handle the URL. We never attempt to send the URL to a Native app here.

If we do have situations where we send intents to Native apps without the user knowing, we should be filing separate bugs.

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: nocrash v0.1

This patch keeps Fennec from crashing with the test page. We display a toast, saying that we could not find an application for this URL.

I tried catching TransactionTooLargeException itself, but Java did not believe the exception was being thrown through the call chain, so I resorted to just use Exception.

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8475955 [details] [diff] [review]
nocrash v0.1

Wes - More eyes in case I am missing something about how these methods are being used.

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

https://hg.mozilla.org/integration/fx-team/rev/41515daa703f

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/41515daa703f

Comment 11

[Wesley Johnston (:wesj)]

Seems sane to me. I would probably have kept the try-catch over a smaller area (hasHandlersForIntent should probably be replaced with a generic "getHandlersForIntent" that just does this system lookup) and added a note about why the try catch was needed. But this works.

Comment 12

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

mfinkle - is this a security issue? based on comment 6 it may not be?

Comment 13

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Curtis Koenig [:curtisk] from comment #12)
> mfinkle - is this a security issue? based on comment 6 it may not be?

I would say it is not a security issue. It is a crash, but I don't think it's a security issue.

Comment 14

[Richard Newman [:rnewman]]

Comment on attachment 8475955 [details] [diff] [review]
nocrash v0.1

Approval Request Comment
[Feature/regressing bug #]:
  None.

[User impact if declined]:
  Crashes with certain malicious input.

[Describe test coverage new/current, TBPL]:
  None.

[Risks and why]: 
  Very low risk: two additional try/catch blocks. Makes some methods safe when they weren't before.

[String/UUID change made/needed]:
  None.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/075e96c2cf12

Comment 16

[Cristina Madaras, QA [:CristinaM]]

Firefox doesn't crash when the URL from comment 0 is open, so I will mark this as verified
Builds:
Firefox for Android 33
Firefox for Android 34 Beta 1
Device:
Asus Transformer Pad TF300T (Android 4.2.1)