Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#3882 closed defect (fixed)

HTTPS Everywhere 1.0 causing crashes on multiple sites

Reported by: jorgev Owned by: pde
Priority: Very High Milestone:
Component: HTTPS Everywhere/EFF-HTTPS Everywhere Version:
Severity: Keywords:
Cc: tom+tor-trac@…, vic.garin@… Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Please refer to this bug:

https://bugzilla.mozilla.org/show_bug.cgi?id=677643

I can confirm that this extension causes easily reproducible crashes in http://www.interviewstreet.com/. This doesn't happen with version 0.9.7, but it happens with 1.0 and 1.0.1 This is in Aurora (future Firefox 8).

It could be Mac OS-only, nobody has confirmed in any other OS.

Child Tickets

Change History (12)

comment:1 Changed 8 years ago by Ver Greeneyes

Similar crashes are also showing up on Linux and Windows:
Linux: https://bugzilla.mozilla.org/show_bug.cgi?id=684167
Windows: https://bugzilla.mozilla.org/show_bug.cgi?id=679471

comment:2 Changed 8 years ago by pde

Per the bugzilla thread, it seems like Mozilla is going to take responsibility for fixing this. However we need to issue an update to the stable branch declaring incompatibility with FF 7 & 8 until this is fixed.

comment:3 Changed 8 years ago by pde

Keywords: wont-fix-here added

comment:4 Changed 8 years ago by StrangeCharm

Cc: tom+tor-trac@… added

comment:5 Changed 8 years ago by pde

Keywords: wont-fix-here removed

comment:6 Changed 8 years ago by pde

Resolution: fixed
Status: newclosed

This should have been fixed by disabling the nsIContentPolicy::shouldLoad / forceURI path in stable 1.0.3.

A similar commit is in git master, and will go out with the next dev release.

Unless this turns out to let some HTTP requests through, this bug will be considered closed for now.

comment:7 Changed 8 years ago by vic

Resolution: fixed
Status: closedreopened

Why do you need to disable the nsIContentPolicy::shouldLoad / forceURI path?

Firefox devs fixed it - See: https://bugzilla.mozilla.org/show_bug.cgi?id=677643

No longer able to crash Firefox Nightly with the latest HTTPS Everywhere dev release and the STR there.

Please restore the nsIContentPolicy::shouldLoad / forceURI path in stable version.

comment:8 Changed 8 years ago by vic

Cc: vic.garin@… added

comment:9 Changed 8 years ago by Ver Greeneyes

https://bugzilla.mozilla.org/show_bug.cgi?id=677643#c35 noted that the cloning FF has to do to work around the crash will cause a performance hit.

https://bugzilla.mozilla.org/show_bug.cgi?id=677643#c47 noted that forceURI() could be avoided on several load types to mitigate the performance hit.

I haven't checked what was actually committed to HTTPS-everywhere, though.

comment:10 in reply to:  7 ; Changed 8 years ago by pde

Resolution: fixed
Status: reopenedclosed

Replying to vic:

Why do you need to disable the nsIContentPolicy::shouldLoad / forceURI path?

Because the Firefox patch isn't in Firefox 4-7, and we'd prefer not to crash those browsers?

Please restore the nsIContentPolicy::shouldLoad / forceURI path in stable version.

Despite Giorgio's concerns, we haven't yet found any cases in which disabling nsIContentyPolicy caused an insecure HTTP load. If we find any, we'll try to turn it back on just for those cases.

comment:11 Changed 8 years ago by pde

Vic, if you can find any cases where we fail to rewrite a request beause of the nsIContentPolicy removal, please report them in #4176 and we can reopen this for real.

comment:12 in reply to:  10 Changed 8 years ago by vic

Replying to pde:

Replying to vic:

Why do you need to disable the nsIContentPolicy::shouldLoad / forceURI path?

Because the Firefox patch isn't in Firefox 4-7, and we'd prefer not to crash those browsers?

The removal was in the main branch of HTTPS Everywhere.

Right now Firefox 4-6 has no security updates.

My suggestion is to keep the nsIContentPolicy::shouldLoad / forceURI path in HTTPS Everywhere v2.0.xDev and release version 2 on the same day that Firefox 8 ships.

Please restore the nsIContentPolicy::shouldLoad / forceURI path in stable version.

Despite Giorgio's concerns, we haven't yet found any cases in which disabling nsIContentyPolicy caused an insecure HTTP load. If we find any, we'll try to turn it back on just for those cases.

Really would appreciate if you left it on for Everything, just in case.

I mean come to think of it, the patch released @ https://bugzilla.mozilla.org/show_bug.cgi?id=677643 for Firefox 8+ is now useless? The patch was because we were using the nsIContentPolicy::shouldLoad / forceURI path right?

Note: See TracTickets for help on using tickets.