[tor-bugs] #30115 [Applications/Tor Browser]: NoScript's XSS popup breaks circuit display in some cases

[Tor Bug Tracker & Wiki]

#30115: NoScript's XSS popup breaks circuit display in some cases
-------------------------------------------------+-------------------------
 Reporter:  gk                                   |          Owner:  tbb-
                                                 |  team
     Type:  defect                               |         Status:
                                                 |  needs_revision
 Priority:  Medium                               |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  tbb-torbutton, tbb-circuit-display,  |  Actual Points:
  TorBrowserTeam201904                           |
Parent ID:                                       |         Points:
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------
Changes (by gk):

 * keywords:  tbb-torbutton, tbb-circuit-display, TorBrowserTeam201904R =>
     tbb-torbutton, tbb-circuit-display, TorBrowserTeam201904
 * status:  needs_review => needs_revision


Comment:

 The code changes look mostly good.

 `let urlOrigin = new URL(origin);` can throw, though, which we don't catch
 anymore (given `let origin = browser.contentPrincipal.origin || '';`). I
 think we should catch that nevertheless, even if we omit log output now.

 That said, consider the following scenario: in tab 1 you open
 https://torproject.org and in tab 2 you open https://blog.torproject.org.
 Checking the circuit display on both tabs should show you the same circuit
 (assume there are no circuit errors like timeouts etc. for the whole
 example). Now, select tab 2 and change the circuit for the blog by
 requesting a new one for that site. What should the circuit display show
 for each tab? With your patch it shows for *both* tabs the new circuit
 used for https://blog.torproject.org. Without your patch only tab 2 shows
 the new circuit while tab 1 still shows the one that got originally used
 to fetch its contents. I think #16936 was concerned with corner cases like
 that where you have a circuit in one tab and a different one for the same
 domain in a new tab. And, yes, I think the current behavior we have (not
 the one coming with your patch) regarding my example above is the one we
 want. Thus, this patch needs revision.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/30115#comment:5>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online