Closed Bug 819554 Opened 7 years ago Closed 7 years ago

Disable warning for sms protocol

Categories

(Firefox for Android :: General, defect)

16 Branch
x86_64
Linux
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 20

People

(Reporter: mfinkle, Assigned: mfinkle)

Details

Attachments

(1 file, 2 obsolete files)

Attached patch patch (obsolete) — Splinter Review
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
Attached patch patch 2 (obsolete) — Splinter Review
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-
Attached patch patch 3Splinter Review
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/mozilla-central/rev/ab25a9cef1c8
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 20
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?
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.