819554 - Disable warning for sms protocol

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

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

Attachment: patch

If web pages use <a href="sms:1254567890">Send me an SMS</a> Fennec will display a "This requires an external application> [OK] [Cancel]" prompt.

We disabled that annoying message in bug 589403 for mailto: and tel: and I think we should remove it for sms: too. The SMS is not automatically sent. The user must still press the "send" button in the SMS app.

Comment 1

[Kevin Brosnan [Ex-Mozilla]]

Do we need to worry about bug 794034 at all?

Comment 2

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

(In reply to Kevin Brosnan [:kbrosnan] from comment #1)
> Do we need to worry about bug 794034 at all?

Probably:
https://viaforensics.com/mobile-security-category/remote-ussd-code-execution-android-devices.html
"The original findings indicated that the factory reset for the Samsung Galaxy S3 can be triggered via an injected frame, QR code, Near Field Communications, or SMS text message."

Comment 3

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

I can add a tel: -like patch for sms: too

Comment 4

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

Attachment: patch 2

This patch adds the same kind of protection used for tel: to sms:

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 690064 [details] [diff] [review]
patch 2

Review of attachment 690064 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/GeckoAppShell.java
@@ +1218,5 @@
> +                if (number.contains("#") || number.contains("*") || uri.getFragment() != null) {
> +                    return false;
> +                }
> +
> +                // Have a special handling for the SMS, as the message body

can we share code between the tel: and sms: checkers? 

In the discussion about how this safety check should work we decided to err on the side of over checking. I suspect that might be adjusted in the future and I'd hate for these to get out of sync. Even more concerning would be if we had to clamp down on some new attack and these got out of sync.

Comment 6

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

Attachment: patch 3

This version creates a helper function to test for the evil codes and uses it for both protocols.

Comment 7

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/ab25a9cef1c8

Comment 8

[Ed Morley [:emorley]]

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

Comment 9

[Ioana Chiorean]

I am not able to reproduce with dump.lassey.us/proto.html on :
Firefox 20.0a1 (2012-12-17)
Device: Galaxy Nexus
OS: Android 4.1.1

Marking bug as VERIFIED FIXED

Comment 10

[Daniel Veditz [:dveditz]]

The approach looks good here