1083794 - SMS Handler should not respond to every kind of message with an SMS

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Wilfred Mathanaraj [:WDM]]

Impact: Multiple attacks; An attacker could buy 1,000 SMS in bulk and send them to a victim, which will then respond with 1,000 expensive SMS. Attack can spoof sender id, and bounce SMS off sender’s device

Fix: Do not respond. The user should know which commands he has enabled and which not. The phone could alternatively just respond to successful commands or keep quiet.

Comment 1

[marta]

Attachment: bug_1083794.patch

Comment 2

[Frederik Braun [:freddy] (OOO - back August 10th)]

Comment on attachment 8507686 [details] [diff] [review]
bug_1083794.patch

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

Rate limiting is a good idea, but I would prefer if the app would only respond to successful commands.

Comment 3

[marta]

The response to any command was a request from the UX.

Comment 4

[Frederik Braun [:freddy] (OOO - back August 10th)]

Well, I guess then we will have to live with rate limiting?! :/

Comment 5

[Frederik Braun [:freddy] (OOO - back August 10th)]

Comment on attachment 8507686 [details] [diff] [review]
bug_1083794.patch

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

I still don't think this is a great idea, but I will not prevent you from doing this.
Clearing the review flag.

Comment 6

[Paul Theriault [:pauljt] (no longer reading bugmail)]

I dont agree that this is an ok solution. In australia, an SMS costs 30c. So I can charge a user 90c an hour, and there is nothing they can do about this right? 

Mike, considering the risk here, is this really a necessary feature for UX?

I am very strongly opposed to feature that is so trivially exploited.

Comment 7

[jachen]

FYI Mike is traveling today and should be able to respond tomorrow. I'm going to needsinfo Hunter as well.

Comment 8

[Hunter Luo]

Needsinfo Message UX owner Jenny, we will discuss internally today.

Comment 9

[Jenny Lee]

Hi Wilfred,

Maybe some of the questions aren't for you but, can you try to provide detail description on: 
1. How does the attack spoof sender id 
2. How does the attack bounce SMS off sender’s device
3. What would be the "respond" from victim's device
4. How to only respond to successful commands

Thanks!

Hi Julien,
I had a discussion with Steve and Bevis but all three of us are pretty clueless about how the attack works as well as what's the patch about, perhaps you will know better :)?

Comment 10

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

From what I see in the patch, the issue is not in the Gaia::SMS application, but in the privacy-panel application, that none of us know :) It's not even in Gaia repository.

Where is this application installed? Who is in charge of this application? Where is the repository for this application?

Comment 11

[Wilfred Mathanaraj [:WDM]]

privacy panel app is currently in development by partner, bugs @ https://bugzilla.mozilla.org/show_bug.cgi?id=1069915 

if you want to have a look at it you can include patches in the bug. Etienne is having a look at the overall privacy panel app.

the question below came from Frederik Braun who did an initial pass from a security perspective. Perhaps he can answer Jenny's questions.

Comment 12

[Frederik Braun [:freddy] (OOO - back August 10th)]

(In reply to Jenny Lee from comment #9)
> Hi Wilfred,
> 
> Maybe some of the questions aren't for you but, can you try to provide
> detail description on: 
> 1. How does the attack spoof sender id 
> 2. How does the attack bounce SMS off sender’s device
> 3. What would be the "respond" from victim's device
> 4. How to only respond to successful commands
> 
> Thanks!
> 

Here's quick recap about the discussion, that hopefully answers your question: The Remote Privacy Protection for the Privacy Panel is a feature that allows a user to control her device through SMS, when it is lost. The user has to set a password in advance and can then ask a friend to send SMS to the lost device for remote control. Commands are "make the phone ring in full volume", "lock the screen", "wipe the device. You issue the commands by sending an SMS like "RPP ring password" or "RPP lock password".

It currently responds to all inbound SMS that start with rpp. It has error handlers for all cases and responds with things like "Command not found" or "Password is not correct". I was concerned that these replies cost money to the user. Especially considering that business customers can buy SMS in bulk and send a lot of SMS for little money to someone with RPP enabled. These premium SMS services also oftenly allow the person buying the SMS in bulk to send with a custom (spoofed) sender address.

Depending on the country, SMS are really expensive. To make this less problematic, Marta suggested rate limiting the replies, i.e. only sending 3 SMS per hour (current patch). This may still mean $1 per hour, which is quite a lot for some people.

I was suggesting that the device alternatively only responds to commands with a successful password. According to comment 3 it was advised by UX that every text should be replied to. This is why we involved you: 

Do you think that replying to only correct passwords is enough?

Comment 13

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

I personally agree with you Frederik. Especially that we took great care in the past to not make the user spend money when it doesn't need to.

We could imagine having a white list of source phone number or contacts (eg: husband/wife/children) where we would always answer. But without this I'd just not answer with an incorrect password.

About the commands: I'd use "RPP <password> <command>" instead of the opposite. Because if we answer only if the password is correct, we can answer an help message if the command is unrecognized but the password is correct.


If we want to keep the rate limiting, then I'd permanently block the source phone number. But I realize this is more work, because then we'd want to be able to unblock it...

Note sure how helpful my comment is.

Comment 14

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

Comment on attachment 8507686 [details] [diff] [review]
bug_1083794.patch

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

::: dev_apps/privacy-panel/js/privacy-panel.js
@@ +264,5 @@
> +
> +    // Limit SMS number per hour to 3.
> +    if (this._timestamps.length > 3) {
> +      return;
> +    }

are there unit tests for this code?

Comment 15

[jachen]

Jenny, can you please respond to the below from Frederik?

> 
> I was suggesting that the device alternatively only responds to commands
> with a successful password. According to comment 3 it was advised by UX that
> every text should be replied to. This is why we involved you: 
> 
> Do you think that replying to only correct passwords is enough?

I feel that sensitivity to cost trumps here- causing the user unnecessary costs will be a big issue if we must respond to every sms.

Comment 16

[Hunter Luo]

After discussion inside UX team related functions. Yes, we agree the cost problem is more important than getting feedback if the password is wrong. Previously our start point was to make sure users get to know they keep entering wrong password and command and don't get any feedback and think this function is broken. But since the concern of spam attack and cost money is more than the responsiveness. We agree to make it only respond success/operation complete message when the password and command is correct. Since the spec is actually owned by partner. So this may require partner to update the UX spec.

Comment 17

[Andrew McCreight [:mccr8]]

Please hide sec-critical bugs.

Comment 18

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

(In reply to Andrew McCreight [:mccr8] from comment #17)
> Please hide sec-critical bugs.

Nothing shipped yet.

Comment 19

[Wilfred Mathanaraj [:WDM]]

and we are not really going to land the code until this is addressed.

Comment 20

[Andrew McCreight [:mccr8]]

Ah, sorry, that wasn't clear from the bug.

Comment 21

[marta]

Attachment: no_response.patch

Patch that provides only response to successful SMS

Comment 22

[Paul Theriault [:pauljt] (no longer reading bugmail)]

The patch looks OK, but its hard to say if its effective at never responding since I am not sure what it applies to? 

I'm looking at: https://github.com/martasect/gaia/tree/privacy-panel-master/dev_apps/privacy-panel/js

Should I be looking somewhere else? (I can't find /js/sms/main.js ) Can you point me in the right direction?

Comment 23

[Kai-Chih Hu [:khu]]

[Blocking Requested - why for this release]:

This is a security critical bug. Based on the blocking criteria(https://wiki.mozilla.org/B2G/Triage#Issues_that_Should_Block), we need to make this one as a release blocker. Thanks.

Comment 24

[bhavana bajaj [:bajaj]]

(In reply to Kevin Hu [:khu] from comment #23)
> [Blocking Requested - why for this release]:
> 
> This is a security critical bug. Based on the blocking
> criteria(https://wiki.mozilla.org/B2G/Triage#Issues_that_Should_Block), we
> need to make this one as a release blocker. Thanks.

Kevin, clearing the nom as this has not shipped yet.

Comment 25

[Al Billings [:abillings - ex-MoCo]]

Can we get an update on when this might be fixed? You have an outstanding review request, Paul.

Comment 26

[Paul Theriault [:pauljt] (no longer reading bugmail)]

This is fixed. No idea in what bug/patch, but I checked at the app no longer replies to every SMS (only if you have the correct code). 

For future reference the code in question can be seen here:
https://github.com/mozilla-b2g/gaia/blob/0f7bb156969c5c838ff90ebc88d7691fc4d94310/apps/privacy-panel/js/sms/main.js#L94

Comment 27

[Christiane Ruetten [INACTIVE]]

Sounds like this was resolved for the 2.1 release, so I'm setting 2.2 to unaffected. Please correct me if I'm wrong.

Comment 28

[Paul Theriault [:pauljt] (no longer reading bugmail)]

(In reply to Christiane Ruetten [:cr] from comment #27)
> Sounds like this was resolved for the 2.1 release, so I'm setting 2.2 to
> unaffected. Please correct me if I'm wrong.

That's correct. And technically this app isn't enabled in production builds anyways.