If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Disable warning for sms protocol

VERIFIED FIXED in Firefox 20

Status

()

Firefox for Android
General
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: mfinkle, Assigned: mfinkle)

Tracking

16 Branch
Firefox 20
x86_64
Linux
Points:
---
Bug Flags:
sec-review +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Created attachment 689921 [details] [diff] [review]
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.
Attachment #689921 - Flags: review?(blassey.bugs)
Do we need to worry about bug 794034 at all?
(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."
I can add a tel: -like patch for sms: too
Created attachment 690064 [details] [diff] [review]
patch 2

This patch adds the same kind of protection used for tel: to sms:
Assignee: nobody → mark.finkle
Attachment #689921 - Attachment is obsolete: true
Attachment #689921 - Flags: review?(blassey.bugs)
Attachment #690064 - Flags: review?(blassey.bugs)
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.
Attachment #690064 - Flags: review?(blassey.bugs) → review-
Created attachment 691349 [details] [diff] [review]
patch 3

This version creates a helper function to test for the evil codes and uses it for both protocols.
Attachment #690064 - Attachment is obsolete: true
Attachment #691349 - Flags: review?(blassey.bugs)
Attachment #691349 - Flags: review?(blassey.bugs) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/ab25a9cef1c8

Comment 8

5 years ago
https://hg.mozilla.org/mozilla-central/rev/ab25a9cef1c8
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 20

Comment 9

5 years ago
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
Status: RESOLVED → VERIFIED
Flags: sec-review?
Keywords: sec-review-needed
The approach looks good here
Flags: sec-review? → sec-review+
You need to log in before you can comment on or make changes to this bug.