Opened 6 years ago

Closed 6 years ago

Last modified 2 years ago

#9507 closed defect (fixed)

Infinite loops in HTTPS Everywhere 2013.8.16 for Chrome

Reported by: pde Owned by: pde
Priority: Immediate Milestone:
Component: HTTPS Everywhere/HTTPS Everywhere: Chrome Version:
Severity: Normal Keywords:
Cc: schoen, micahlee Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

The recent release of HTTPS E for Chrome only works until the user restarts their browser. After that, it blocks page loads.

Child Tickets

Change History (20)

comment:1 Changed 6 years ago by pde

I've unpublished the extension (and also deleted the updates.xml file off our server for people who install from eff.org rather than the web store), but I can't re-push an old release because I don't have access to the signing key while travelling.

comment:2 Changed 6 years ago by pde

We got several reports of this by IRC. I was able to reproduce with the extension from the chrome web store. So far as I can tell, the biggest suspect is that it seems like 2013.8.16 was released from the master branch rather than 3.0, so it has a lot of minimally tested rulesets in it.

However I haven't yet been able to reproduce using the extension from eff.org, so I'm not actually sure if that's the same file as the web store was sending to people (although it really should be).

comment:4 Changed 6 years ago by pde

This seems to be a heisenbug; I can't always reproduce it :(.

My best theory about what's going on is that the massively expanded ruleset library in master combined with the lack of this fix in master meant that we could really churn on checking rulesets in some cases. That would be most plausible if there are some global rulesets in master...

comment:5 Changed 6 years ago by pde

Summary: HTTPS Everywhere 2013.8.16 breaks ChromeReleasing HTTPS Everywhere 2013.8.16 from the master branch seems to break Chrome

comment:6 Changed 6 years ago by realityexists

It's 100% reproducible for me. Let me know if I can help by enabling some kind of logging and posting the output.

comment:7 Changed 6 years ago by pde

realityexists: if you go to chrome://extensions , enable developer mode, and then click on Inspect views: _generated_background_page.html , then click on "Console", you should get a debug log. Can you paste as much of that as you can get while the problem is occurring?

comment:8 Changed 6 years ago by pde

Meanwhile: Micah + Seth, I have signed a tag for a chrome-2013.8.17 bugfix release in the stable branch of my github remote, https://github.com/pde/https-everywhere.git

Unfortunately I can't push to the official repo from my travel laptop. However you should be able to merge from there and just release the chrome-2013.8.17 tag as is.

comment:9 Changed 6 years ago by realityexists

Cc: tor-trac@… added

When I first enable the extension and click on a tab I get log entries like this:

Applicable rules for toolbarqueries.google.com: util.js:7
  Search www.google.com util.js:7
  Google Search ... <snipped>

but I don't think these are related. When I actually enter a URL and the "Waiting for extension HTTPS Everywhere" toolbar message appears there are no new log entries.

comment:10 Changed 6 years ago by realityexists

Looks like I need to have another extension enabled to reproduce the problem: Flag For Chrome 0.4.1 https://chrome.google.com/webstore/detail/flag-for-chrome/dbpojpfdiliekbbiplijcphappgcgjfn

Starting with all extensions disabled, if I enable only HTTPS Everywhere I can load a page. As soon enable Flag For Chrome the following messages are logged: http://pastebin.com/upPnC4F9 If I then try to load a page the "Waiting for extension HTTPS Everywhere" toolbar message appears and the page does not load. Even if I then disable Flag for Chrome the problem continues to occur until I disable HTTPS Everywhere.

comment:11 Changed 6 years ago by cypherpunks

On my side even if the only extension installed at all is HTTPS Everywhere this still happens. After it throttles a while eventually chrome://extensions/ responds and I can disabled it. Then I can load any pages - re-enable it during the running session, and it's fine. Next load, same dance.

comment:12 Changed 6 years ago by backflash

Same thing. 2013.8.16 _generated_background_page.html console is looping "excluded uri http://www.google.ca/0.xxxxxx/0.xxxxxxx" when going to http://www.google.ca/[insert anything here]. Was working fine the day before 2013.8.16. I tracked down the "console print" line and there are many loops that can be encountered to that point in the source code. I'm auditing it right now to see the source of this issue.

comment:13 Changed 6 years ago by backflash

rules.js Line 257:

for (i = 0; i < rs.length; ++i) {

Conflict with with sub loop

for(i = 0; i < this.exclusions.length; ++i) {

Change to use "var" to avoid rest of global variable I back to 0. There is the source of your loop. Tested working fix by

Changing:
for(i = 0; i < this.exclusions.length; ++i) {
To
for(var i = 0; i < this.exclusions.length; ++i) {

Has been tested as working. I advise (and it's good practice) that var is added to every "for loop" to avoid this issue the future.

Lines I changed:
43
50
166
257
166
176
206
211
……..

I don't see how to attach patches here. So here is the fixed file: http://pastebin.com/nXrFxvU7

comment:14 Changed 6 years ago by cypherpunks

The new https-everywhere-2013.8.17.crx release seems to solve the problem on my three systems.

Is this pde's fix and backflash or just the former? Out of curiosity because for now I'm going to disabled auto-updating unless another one seems imminent. Thanks.

comment:15 Changed 6 years ago by backflash

Tested HTTPS Everywhere 2013.8.17 on chrome store. Verified working.

comment:16 Changed 6 years ago by zyan

pde's fix alone fixed it for me.

comment:17 Changed 6 years ago by realityexists

2013.8.17 seems to work OK for me, too.

comment:18 Changed 6 years ago by realityexists

Cc: tor-trac@… removed

comment:19 Changed 6 years ago by pde

Resolution: fixed
Status: newclosed
Summary: Releasing HTTPS Everywhere 2013.8.16 from the master branch seems to break ChromeInfinite loops in HTTPS Everywhere 2013.8.16 for Chrome

backflash, the bug was triggered by accidentally releasing from the git master rather than stable branch. Which doesn't explain why the bug was only visible in the master branch, because I think it's in stable too. Probably, there were just larger numbers of rulesets and exclusions in master, which exposed the loop variable clobbering problem that you found. Also, this patch was in stable but not in master, and it caused many rulesets to be checked twice on a given request (in fact you can see that happening in the logs that realityexists pasted above.

I'm going to apply your patch to stable and master.

comment:20 Changed 2 years ago by teor

Severity: Normal

Set all tickets without a severity to "Normal"

Note: See TracTickets for help on using tickets.