552124 - "undo history" in urlbar exposes urls visited while in private browsing

Status: RESOLVED FIXED

(Firefox :: Private Browsing, defect)

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Steps to reproduce

1. Browse to a few different websites
2. Put focus in the url bar
3. Press platform-specific "undo" key combination (ctrl-z on windows, cmd-z on
   mac)
4. Notice that you can see the URLs of the websites visited in step 1
5. Enter private browsing mode
6. Visit a few websites
7. Exit private browsing mode
8. Repeat step 2
9. Repeat step 3
10. Notice that you can see websites visited while in private browsing mode in
    step 6.


So what is happening here is that our normal undo functionality "remembers" all visited websites. And it is agnostic to entering and exiting private browsing mode.

One fix would be to simply clear the undo history when private browsing mode is exited from all open url-bars. Though to be honest I wonder if we should *always* clear the history when a new page is opened.

I'm not sure if this bug needs to be hidden or not. Leaving that up to the security folks. It does sound bad if people start running around inspecting each others private browsing history.

Brandon recommended that this be marked sg:low as that is where we usually put privacy bugs.

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Oh, and to be clear, in steps 3 and 9 above, you'll probably have to "undo" multiple times.

Comment 2

[(no longer active)]

Attachment: Patch (v1)

So, I could just fix this for the private browsing case, but I don't think that the current behavior actually makes any sense.  I think that conceptually, the location bar belongs to the page being displayed, so things like changing tabs or going to another link should not leave traces inside the location bar.

With this patch, the undo command is available when the user is manipulating the contents of the location bar, but clears the history when a new URI is set.  This will take care of the private browsing problem as well.

Johnath recommended me to ask for Gavin's review to check with him if this behavior change is acceptable.

Comment 3

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #2)
> I think that conceptually, the location bar belongs to the page being
> displayed, so things like changing tabs or going to another link should not
> leave traces inside the location bar.

I don't really agree... the URL bar is conceptually related to the tab rather than the page being viewed, and I see no reason to clear undo history on normal page navigation. I think I would prefer a private-browsing specific solution.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Personally I think it's extremely weird that switching back and forth between two tabs means that I build up undo history for the url bar.

But I'm less interested in the UI design issue, more interested in getting the privacy issue fixed.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 432630 [details] [diff] [review]
Patch (v1)

per comment 3

Comment 6

[Benjamin Smedberg]

This bug is currently marked blocking 1.9.3alpha4, which was originally scheduled for Monday. If it cannot land today, does it still need to block the devpreview schedule?

Comment 7

[(no longer active)]

Crafting a patch right now.  If I can get a quick review, this can land today.

Comment 8

[(no longer active)]

Attachment: Patch (v2)

Updated patch according to comment 3.

Comment 9

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/c87e62542e1d

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 436280 [details] [diff] [review]
Patch (v2)

Approved for 1.9.2.4 and 1.9.1.10, a=dveditz for release-drivers

Checking in the test gives away the bug. Please hold off on checking in tests for hidden bugs (or else does that mean the bug is incorrectly hidden?).

Comment 11

[:Gavin Sharp [email: [e-mail]]]

I don't think there's any point to hiding bugs about flaws with the private browsing functionality, in general. I think that knowing its limitations is just as (or perhaps even more, given Bugzilla's audience) likely to help potential victims than it is to help attackers.

So I'd vote to just open this bug.

Comment 12

[(no longer active)]

(In reply to comment #10)
> Checking in the test gives away the bug. Please hold off on checking in tests
> for hidden bugs (or else does that mean the bug is incorrectly hidden?).

I think it's in fact incorrectly hidden, in my opinion.  It is not a security issue, it's only a privacy issue, and we've had a lot of privacy bugs in private browsing which were openly filed, discussed and fixed.  Anyway, I'll hold off on checking this in to branches until either it's opened by a member of the security group, or we're close to the code freeze for branches.

Comment 13

[:Gavin Sharp [email: [e-mail]]]

(opening)

Comment 14

[(no longer active)]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/59468c78d5fc
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/2a045307fee2

Comment 15

[(no longer active)]

hg transplant messed up part of my checkin on 1.9.1.  Landed the rest of the patch in:

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/455da0aa72f3

Comment 17

[Al Billings [:abillings - ex-MoCo]]

Verified for 1.9.1 and 1.9.2 via passing automated test.