1161545 - HTML injection in Communications app

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Dialer, defect)

Description

[Muneaki Nishimura]

Attachment: vulnerability 1

User Agent: Mozilla/5.0 (Windows NT 6.2; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2383.0 Safari/537.36

Steps to reproduce:

1. Open "Browser" app and download vCard from http://mallory.csrf.jp/fxa/xss.vcf
2. Launch "Contacts" app and open "Settings" page.
3. Push "Import Contacts" button and choose "Menory card"
4. Repeat step 2 & 3 and push "Done" button in the Setting menu
5. "Contacts" list page is shown on the screen
6. Choose one of the contact list entry starting with "<s>FIRSTNAME</s>..."
7. Edit the entry by pushing edit icon on the right-top on the screen
8. Update company name e.g., abc and push "Update" button on the right-top button
9. Here, you can show the "Duplicates found" popup and then the HTML tags in the FIRSTNAME and LASTNAME are not properly escaped and it works. (vulnerability 1)
10. Return to the home screen and start "Phone" app
11. Push "999" on the screen, then the candidates contact entries are displayed on the screen
12. However, HTML tags in phone number displayed automatically is not  are not properly escaped and it works. (vulnerability 2)



Actual results:

In vulnerability 1 & 2 in above procesure, there are two HTML injection possibilities.


Expected results:

In vulnerability 1 & 2 in above procesure, HTML tags should be escaped properly.

Comment 1

[Muneaki Nishimura]

Attachment: vulnerability 2

Comment 2

[Kevin Grandon :kgrandon]

Attachment: [gaia] Pull Request - Fix contact matching item escaping

Hey - I can help out here.

Fernando, Francisco - Here is a first patch which will solve the escaping for the list items in the contact matching page.

The approach is to use the template strings as suggested by our security guys. I can also fix the rest of the issues in this bug assuming you are ok with this approach. We should probably consider phasing out the template.js file in favor of using tagged template strings. Please take a look and let me know what you think. Thanks!

Comment 3

[Kevin Grandon :kgrandon]

Assigning to myself for now. I'll help out with some of these issues.

Comment 4

[Kevin Grandon :kgrandon]

Attachment: [gaia] Pull Request - Fix dialer suggestion escaping

Doug or Etienne - could either of you guys review this? Thanks!

Comment 5

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8602597 [details] [review]
[gaia] Pull Request - Fix contact matching item escaping

Tried on the phone and working for me.

Thanks Kevin!

Comment 6

[Etienne Segonzac (:etienne)]

Comment on attachment 8602613 [details] [review]
[gaia] Pull Request - Fix dialer suggestion escaping

Thanks!

My 2 cents:
I think we should lazy load `tagged.js` with the `suggestion_bar.js` file. And there might be a follow up to file about the call history and the callscreen app.

But mainly, redirecting to Gabriele as a heads-up :)

Comment 7

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8602613 [details] [review]
[gaia] Pull Request - Fix dialer suggestion escaping

Ouch, I didn't realize we were using innerHTML for this. This sounds like a good candidate for a healthy refactor (well, large swathes of the dialer code do).

Comment 8

[Kevin Grandon :kgrandon]

(In reply to Etienne Segonzac (:etienne) from comment #6)
> Comment on attachment 8602613 [details] [review]
> [gaia] Pull Request - Fix dialer suggestion escaping
> 
> My 2 cents:
> I think we should lazy load `tagged.js` with the `suggestion_bar.js` file.
> And there might be a follow up to file about the call history and the
> callscreen app.

Thanks for taking a look. This makes sense and I've gone ahead and made the change as requested.

Comment 9

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8602613 [details] [review]
[gaia] Pull Request - Fix dialer suggestion escaping

Looks good, though Gabriele's review is enough here.

Comment 10

[Kevin Grandon :kgrandon]

In master:
https://github.com/mozilla-b2g/gaia/commit/e13f5b2ae166dc7d9d8d9f3833224a2e08415b2b
https://github.com/mozilla-b2g/gaia/commit/f4fcb8a937826ee0e7f79b5eee3c48b547b1cef1