543922 - Strip usernames and passwords (and sanitize file://) in URLs submitted with crash reports

Status: RESOLVED INCOMPLETE

(Camino Graveyard :: General, defect)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Because of the low number of URLs captured, it only takes a crash (or two) for a URL to make it to our Topcrash-by-URL report.  This means that if someone crashes on a URL that includes a username or password, that information is exposed publicly.  Also, if a user crashes on a local (file://) URL, there's a good chance their username is going to be exposed.

We need to make sure that we strip usernames and passwords from URLs when we store them in advance of Breakpad sending them.  We should probably also not send file:// URLs or sanitize them somehow, too.

There are live cases of both of these on crash-stats right now (I've filed bug 543921 to have Socorro sanitize things on their end, and reed is working in IT to get the live cases cleaned up there), so filing this as security-sensitive to keep it hidden until at least the current reports are sanitized.

Comment 1

[Reed Loden [:reed]]

ted, should we also do this for the general breakpad stuff (not just Camino)?

Comment 2

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #0)
> We need to make sure that we strip usernames and passwords from URLs when we
s/We need to make sure/We should try if at all possible/; I don't know if that's possible the way the code works or not.

Socorro shouldn't leak them, but if we can not send the critical parts to begin with, that would be better.

(In reply to comment #1)
> ted, should we also do this for the general breakpad stuff (not just Camino)?

The guessing-what-URL-user-was-on-when-crashed code is per-product AIUI, so if we want to bulletproof the client as well as Socorro, probably either each product needs to ensure that its URL-guessing code doesn't leak this to the crashreporter, or the crashreporter needs to do sanitizing of URLs before sending them.

Comment 3

[Stuart Morgan]

Yes, we can do whatever string processing we want in setReportedURL: to filter at the Camino side.

We should definitely do that, but I'm surprised by this being an issue since I thought there was a lower-limit on how many times a URL had to crash before it could make the topcrash report.

Comment 4

[(not currently active) Ted Mielczarek]

I don't really see the benefit as long as Socorro is fixed to not display sensitive URLs publicly. There are plenty of ways for a URL to contain sensitive information, which is why we explicitly allow opt-out of sending URLs in the crash reporter.

Comment 5

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #4)
> There are plenty of ways for a URL to contain
> sensitive information, which is why we explicitly allow opt-out of sending URLs
> in the crash reporter.

Neither here nor there for this bug, but opt-out of URL submission is only effective in preventing the disclosure of private info in URLs if

1) The user is "paranoid" and never submits any URLs, or
2) The user knows he crashed on a page whose URL contains private info *AND* the URL-guessing code is always 100% accurate (so the user chooses to opt-out for this crash).

Comment 6

[(not currently active) Ted Mielczarek]

Yes, but my point is that you can also have private data in URL parameters, or the hostname or path (perhaps an unannounced product or something like that), so we really can't actually sanitize all URLs, so I think just fixing this specific case server-side is good enough.

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Since Socorro has mitigated this on the crash-stats end, dropping severity and minusing for 2.0.2, though I still think we should sanitize what we can before sending.

Comment 8

[Josh Aas]

Does this discussion need to be security-sensitive? Can this issue be resolved?

Comment 9

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to Josh Aas (Mozilla Corporation) from comment #8)
> Does this discussion need to be security-sensitive?

I don't know.

> Can this issue be resolved?

In comment 3, Stuart still wanted us to sanitize what we sent to Socorro; neither he nor I have yet made that change to our pass-this-URL-to-Breakpad code, so the bug is still valid and awaiting a fix.

Comment 10

[Daniel Veditz [:dveditz]]

unhiding given comment 7