rules with [^/@:] don't catch all traffic
Torproject.xml currently has the following
<rule from="^http://([^/:@]*)\.torproject\.org/" to="https://$1.torproject.org/"/>but an attacker trying to get you to send (for example) cookies in the clear can just include a username part in (for example) an img src to coax the browser into making a cleartext connection:
<html>
<head>
<title>a test</title>
</head>
<body>
<!-- this first one gets loaded in the clear -->
<img src="http://www@www.torproject.org/images/icon-default.jpg" />
<!-- https-everywhere intercepts this one and sends it out over https -->
<img src="http://www.torproject.org/images/icon-default.jpg" />
</body>
</html>this seems especially bad for sites with cookies to project which don't have the secure flag set properly.
Activity
Yes, i'm sure. visiting the URLs directly will trigger firefox's confirmation prompt, but i'm concerned more about the embedded img src's which don't seem to be prompted for.
I've placed the following code online:
<html> <head> <title>a test</title> </head> <body> <!-- this first one gets loaded in the clear --> <img src="http://www@duckduckgo.com/nduck.v104.png" /> <!-- https-everywhere intercepts this one and sends it out over https --> <img src="http://duckduckgo.com/nduck.v104.png" /> </body> </html>If you have firebug installed, open up the net console, and visit the example (The net console might close when you switch domains. just re-open it and refresh the page with ctrl-shift-R)
you should see one request to the duckduckgo servers in the clear (HTTP) and another one encrypted (HTTPS).
tcpdump + wireshark confirms this behavior for me on a debian squeeze system with https-everywhere 0.9.0 installed and the duckduckgo rule enabled.
So what can we do about this. Here are some ideas:
-
Ask mozilla to raise the warning prompt for images and other subsidiary requests.
-
Take the replace [^/:@] with [^/]. I think that defeats dkg's attack. Ironically it would leave all the rules that DON'T start with a pattern vulnerable. We would need to add a pattern to the front of every (www.)? rule to catch a username/password :(.
-
Use Mozilla's built in URI parsing to strip out username/password fields before we do URI rewriting (then add them back in, if we think they're ever legit?).
-
Per rransom's suggestion, move to something like agl's proposed chromium syntax. https://mail1.eff.org/pipermail/https-everywhere/2010-November/000545.html. There are several downsides to that.
Trac:
Priority: normal to major-
-
In trac wiki syntax, you can get literal strings to display properly by wrapping them in backtick characters. So:
replace `[^/:@]` with `[^/]`.becomes:
replace
[^/:@]with[^/].I really hope we don't go with pde's option 1. i don't think that mozilla's warning prompt in these cases is particularly useful or intelligible.
Replying to pde:
- Per rransom's suggestion, move to something like agl's proposed chromium syntax. https://mail1.eff.org/pipermail/https-everywhere/2010-November/000545.html. There are several downsides to that.
The only downside is that you will need to convert all of the existing rulesets to the new format. This time, add an XML namespace URI and/or some other version indicator.
But the real reason this is necessary is (quoting agl's message):
Serialising and re-parsing URLs is very scary from a security point of view. It would be greatly preferable to handle URLs in their processed form.
If we don't start operating on parsed URLs, we can only expect more exploitable bugs like this one in the future.
Since Firefox extensions can use arbitrary JavaScript code to munge URL requests before they are acted on, HTTPS Everywhere rules can easily continue to support matching URL components against regular expressions and inserting captured strings into any component of the new URL.
Replying to rransom:
The only downside is that you will need to convert all of the existing rulesets to the new format. This time, add an XML namespace URI and/or some other version indicator.
Yes, and the fact that the Wikipedia and Google Search rulesets cannot be represented with fewer than thousands of entries in agl's format. Yes, we could do interfield regexps of some sort, but only at the expense of significant added complexity.
But the real reason this is necessary is (quoting agl's message):
Serialising and re-parsing URLs is very scary from a security point of view. It would be greatly preferable to handle URLs in their processed form.
If we don't start operating on parsed URLs, we can only expect more exploitable bugs like this one in the future.
So my proposal #3 (closed) is a hybrid between these approaches; it relies on Mozilla to do some but not all of the URI parsing. Question: can we think of any other categories of parsing trouble that we might run into if we do #3 (closed)?
The solution 3 fix is (allegedly) in git:
Trac:
Status: accepted to needs_review