749519 - The web runtime does not indicate web forgery errors to users on URLs flagged as web forgery

Status: RESOLVED WONTFIX

(Firefox Graveyard :: Webapp Runtime, defect, P1)

Description

[Jason Smith [:jsmith]]

Steps:

1. Install an application that references URL flagged as web forgery
2. Launch the application

Expected:

An error should sent to the user indicating that the URL as been reported as web forgery - Blocked as a result, based on security preferences.

Actual:

No error is reported - The user is sent directly to the URL.

Additional Notes:

Tested by creating a fake app pointing to http://www.mozilla.org/firefox/its-a-trap.html. The error was not reported within the runtime, but reported in firefox.

This is a security issue, as this makes the user vulnerable to phishing (http://en.wikipedia.org/wiki/Phishing).

Comment 1

[Jason Smith [:jsmith]]

Attachment: FF vs. WebRT Web Forgery

Comment 2

[:Gavin Sharp [email: [e-mail]]]

This might just be a matter of setting the relevant browser.safebrowsing.* prefs.

One potential hiccup: we have agreements with the safebrowsing provider (Google) that covers our usage of the service. We might want to double check that use from within the web app runtime is covered.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: enables phishing/malware warnings

Here's a patch that enables the component that manages the lists of phishing/malware URLs via preferences and component initialization on startup.

Some part of the platform then uses those lists to prevent a URL load and display a modal dialog when you try to load a phishing/malware page from within the browser.

This appears to work, even though the component that manages the lists of phishing/malware URLs is in browser/components/safebrowsing/, presumably because browser components live in the GRE components directory.  But bsmedberg plans to change that, at which point I suppose the component will have to move to toolkit to remain accessible to the runtime.

The modal dialog doesn't seem like an ideal interface, but it does at least protect users from these attacks.  I'd like to iterate on a better one, like the one that Firefox has, but it seems worth landing this minimal fix in the meantime.

Comment 4

[Myk Melez [:myk] [@mykmelez]]

To test, I used this test app:

http://mykzilla.org/app/

Press the "install app" button to install it, then start the app and click on the phishing and malware links.  You may need to wait some time after starting the app before clicking on the links to let the app download the lists of phishing/malware links (not sure how long it takes).


(In reply to :Gavin Sharp (use [e-mail] for email) from comment #2)
> One potential hiccup: we have agreements with the safebrowsing provider
> (Google) that covers our usage of the service. We might want to double check
> that use from within the web app runtime is covered.

Yup, I'll look for someone to make this determination.

Comment 5

[Benjamin Smedberg]

It's not clear to me why we actually need to use the safebrowsing service from within a webapp which is supposed to be a single domain only; it doesn't have the same risk factors as normal browsing. What is the size of the safebrowsing database? Replicating that independently across each webapp could become costly both in terms of network and in terms of disk space.

Comment 6

[Jason Smith [:jsmith]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #5)
> It's not clear to me why we actually need to use the safebrowsing service
> from within a webapp which is supposed to be a single domain only; it
> doesn't have the same risk factors as normal browsing. What is the size of
> the safebrowsing database? Replicating that independently across each webapp
> could become costly both in terms of network and in terms of disk space.

Note - Apps intend to run on a single domain, but does not mean they actually do that. Many apps I've looked at do allow access and use within an app on a different domain other than the app, since loading external links within a web app that are not target blank load within the app.

Comment 7

[Myk Melez [:myk] [@mykmelez]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #5)
> It's not clear to me why we actually need to use the safebrowsing service
> from within a webapp which is supposed to be a single domain only; it
> doesn't have the same risk factors as normal browsing.

Webapps aren't constrained to a single domain, although only their origin domain gets extra permissions (like appcache, indexeddb, fullscreen) by default.

The safebrowsing service is helpful in two cases: 1. a malevolent app at a safe origin/launch_path intentionally loads an unsafe page; 2. a benevolent app at a safe origin/launch_path is tricked into loading an unsafe page.

One way the latter case can occur is that the benevolent app loads an untrusted page into an iframe or new window, and that page changes the location of the top/opener window.

At the moment, it isn't possible to protect against this, although <iframe sandbox> and/or <mozbrowser> will enable apps to protect themselves against the iframe variant of the problem in the future; and Apps folks have talked about adding an "allowed_origins" field to app manifests that restricts the origins to which an app window can be changed, which would mitigate the problem for both iframes and new windows.


> What is the size of
> the safebrowsing database? Replicating that independently across each webapp
> could become costly both in terms of network and in terms of disk space.

I'm not the expert here, and I don't know how much information is involved, but safebrowsing appears to use the Places database, which starts at 10MB and didn't grow for my test app when populated with safebrowsing information.

We plan to reduce the size of the Places database, however (bug 762083), in which case safebrowsing data may have more of an impact.  And in any case, I generally agree with you that replicating this data for each app could become costly, especially as desktop webapps become more popular, and the average number of apps that a typical user has installed increases.

But I see a fix like this as a short-term solution that gives us valuable protection at a reasonable cost and buys us time to evaluate and address resource consumption issues in the medium term, f.e. by sharing this data between app/Firefox profiles, for which cf. bug 763812 and Windows 8 Integration - Big Issues <https://wiki.mozilla.org/Firefox/Windows_8_Integration#Big_Issues>.

Comment 8

[Benjamin Smedberg]

Comment on attachment 635530 [details] [diff] [review]
patch v1: enables phishing/malware warnings

Redirecting review, I have neither the relevant expertise nor the time.

Comment 9

[Justin Dolske [:Dolske]]

(In reply to Myk Melez [:myk] [@mykmelez] from comment #7)

> > What is the size of
> > the safebrowsing database? Replicating that independently across each webapp
> > could become costly both in terms of network and in terms of disk space.
> 
> I'm not the expert here, and I don't know how much information is involved,
> but safebrowsing appears to use the Places database, which starts at 10MB
> and didn't grow for my test app when populated with safebrowsing information.

Nope, this is in the local cache/profile, /safebrowsing and urlclassifier.*.

In my well-worn profile, that's about 67MB of data. D:

The amount of data transferred has been a concern to Google in the past (istr some of the protocol revisions were to minimize the volume/frequency of updates)... So slurping down extra sets of data would certainly be of concern to them. Each webapp has a separate profile, right? Yikes. You'll definitely need to check with Partners/Legal (kev? harvy?) to see if that's ok. And it would likely need a pass through Privacy, since I think there's a per-client cookie/ID that SB uses. That could leak some degree of webapp usage to Google.

I fear, though, that just the local overhead of 1 DB per app is going to be a non-starter. Like, what's mobile/B2G going to do about that on mobile?

CCing gcp, since he worked on the backend of this recently to make it usable on mobile... ala bug 673470. How hard would it be to make this data cross-process shareable? That would seem like it would eliminate both concerns (local overhead, google overhead).

Comment 10

[Justin Dolske [:Dolske]]

Comment on attachment 635530 [details] [diff] [review]
patch v1: enables phishing/malware warnings

_Functionally_ I think this patch is ok (assuming it works ;). But minusing pending out resolution of the above issues.

Comment 11

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 635530 [details] [diff] [review]
patch v1: enables phishing/malware warnings

(In reply to Justin Dolske [:Dolske] from comment #9)
> I fear, though, that just the local overhead of 1 DB per app is going to be
> a non-starter. Like, what's mobile/B2G going to do about that on mobile?

This bug is desktop-specific, so it doesn't affect what we do on Android/B2G.

Nevertheless, 67MB of data per app feels like too much on desktop too, even as a stopgap solution, so let's figure out how to share this information between webapps (and Firefox).

Comment 12

[Gian-Carlo Pascutto [:gcp]]

>This bug is desktop-specific, so it doesn't affect what we do on Android/B2G.

I don't really see why from the description, or is that because WebRT support isn't far along enough in Android/B2G to be relevant?

>CCing gcp, since he worked on the backend of this recently to make it usable on 
>mobile... ala bug 673470. How hard would it be to make this data cross-process 
>shareable? That would seem like it would eliminate both concerns (local overhead, 
>google overhead).

The data is a set of flat files that are read sequentially and written out sequentially. You need to update them and be crash resilient (Bug 727370) when doing so.

Accessing them from multiple places would mean you negotiate one of the accessing processes as the one that is doing the actual updates. The others can just read in the .pset files (they're the only ones that you need for checking). But if the updating process exists (or crashes), the others need to pick that up and negotiate a new updater. It's vital that the data is never stale.

I don't think that's easy to get right.

The rewrite in bug 673470 reduces the database to 4.5M, but unfortunately I had to back it out in a hard reminder that our SafeBrowsing tests right now don't cover the real server communication sufficiently (Bug 744993). (Dolske, this is why I advised caution in your clean-up bug)

I am currently working on debugging that part, and will eventually add more tests that also cover more of the communication with the server. To be honest, while this is still in progress it's perhaps not the best time to go all out on changing the SafeBrowsing/UrlClassifier service.

Comment 13

[Myk Melez [:myk] [@mykmelez]]

(In reply to Gian-Carlo Pascutto (:gcp) from comment #12)
> I don't really see why from the description, or is that because WebRT
> support isn't far along enough in Android/B2G to be relevant?

The runtimes for desktop, Android, and B2G are separate and independent features/codebases, and even when we intend for them to behave consistently in a particular respect (such as flagging web forgery errors), the UI affordances by which they do so are likely to be different, and the implementations are certain to be.

So bug reports tracking work on the runtimes are generally runtime-specific, as in this case, which is about implementing web forgery indications in the desktop runtime.

It is, of course, valuable to coordinate the work we do so the runtimes present a consistent model for apps where it is appropriate to do so.  But we tend to do that coordination outside of the context of individual bug reports, in discussion forums like dev-webapps and meetings like the Apps Engineering meeting.

(There are also some cross-runtime coordination efforts taking place within Bugzilla, like k9o tracking flags and bugs in the Tracking::Kilimanjaro component.  One could, for example, imagine a "user story" bug in that component called "users get notified when an app tries to navigate to a page identified as a Web Forgery or Attack Site" that depends on runtime-specific bugs like this one).


> The data is a set of flat files that are read sequentially and written out
> sequentially. You need to update them and be crash resilient (Bug 727370)
> when doing so.
> 
> Accessing them from multiple places would mean you negotiate one of the
> accessing processes as the one that is doing the actual updates. The others
> can just read in the .pset files (they're the only ones that you need for
> checking). But if the updating process exists (or crashes), the others need
> to pick that up and negotiate a new updater. It's vital that the data is
> never stale.
> 
> I don't think that's easy to get right.

Agreed, it's a hard problem to solve.


> The rewrite in bug 673470 reduces the database to 4.5M, but unfortunately I
> had to back it out in a hard reminder that our SafeBrowsing tests right now
> don't cover the real server communication sufficiently (Bug 744993).
> (Dolske, this is why I advised caution in your clean-up bug)

That's still significant, but it's small enough to consider spending for each app in the short term (on desktop).

Comment 14

[Jason Smith [:jsmith]]

Per the last triage on Wednesday, we don't think that this needs to be fixed for FF 16 (aka uplifted for Aurora). Flagging as such.

Comment 15

[sandeep]

[Blocking Requested - why for this release]:

Comment 16

[Myk Melez [:myk] [@mykmelez]]

Per bug 1238079, we're going to disable the desktop web runtime and remove it from the codebase, so we won't fix these bugs in it.