Browser-laptop: Context menu for page can access details for another tab (in this case, LastPass credentials)

[jonathansampson]

Test plan

https://github.com/brave/browser-laptop/pull/8363#issue-222240660

---

• Did you search for similar issues before submitting this one?
  Yes

• Describe the issue you encountered:
  When attempting to use LastPass in a private tab, the credentials available in the context menu are those from the most recently visited non-private tab.

• Platform (Win7, 8, 10? macOS? Linux distro?): Windows 10

• Brave Version (revision SHA): https://github.com/brave/browser-laptop/commit/10539c2316a1b9a8011aa5fab742957d8a0b509e

• Steps to reproduce:

  1. Navigate in a public tab to a site that uses LastPass credentials
  2. Navigate in a private tab to _another site_, and check LastPass in context menu
  3. LastPass will offer the credentials for the public site

• Actual result:
  Our context menus are confused about which site the user is on.

• Expected result:
  Our context menus are aware of where the user is, and will not offer credentials for any other domain.

• Will the steps above reproduce in a fresh profile? If not what other info can be added?
  N/A

• Is this an issue in the currently released version?
  N/A

• Can this issue be consistently reproduced?
  Yes

• Screenshot if needed:
  

• Any related issues:

[diracdeltas]

i can't repro this on master, but supposedly lastpass should not be enabled at all in private tabs

[diracdeltas]

i think this is blocked on someone (@bridiver?) confirming what the expected behavior in private tabs is. IMO lastpass should be disabled in private tabs until https://github.com/brave/browser-laptop/issues/7907 is done

[bsclifton]

++ on what @diracdeltas said (we should disable in private tabs until #7907 is done)

I'll try reproducing w/ Preview 3 on macOS and Windows and will report back

[bridiver]

the expectation is that lastpass shouldn't work at all right now in private tabs so I'm not sure why it does anything at all. @jonathansampson what is the incognito setting in the manifest?

[diracdeltas]

@bridiver i believe it's the default spanning

[bridiver]

I believe the only problem here is that we're showing the context menu when we shouldn't be. I can't get lastpass to do anything in a private tab without using the context menu. All extension context menus should be disabled in private tabs until #7907 is done

[diracdeltas]

has anyone verified that the lastpass content scripts are not running in priv. tabs?

[bridiver]

they show up in private tabs, but so do the content scripts for 1pw and they don't do anything

[diracdeltas]

IIRC lastpass/1pw run content scripts to do autofill without a context menu in some cases

[jonathansampson]

I was unable to get LastPass to fill any forms in a private tab. This could be nothing more than preserving the context menu state. Another thing to check for would be the browserAction badge changing when entering a private tab.

I'm out of the office right now, but can check a. It more in about an hour.

[bsclifton]

@jonathansampson can you check if this is an issue in our current shipped version (0.14.1)? If so, I think we can unassign ourselves and push this back

[jonathansampson]

@bsclifton Yes. This was/is an issue in 0.14.1. Also worth noting that this appears to be an issue with the context menu alone. Actually opening the LastPass popup window reveals no confusion. One alarming issue is that the user can copy credentials for Domain A while they're in a private instance of Domain B. So that's worth tracking.

[bsclifton]

awesome, thanks for the detailed info @jonathansampson! 😄

I'm going to push this back- I set a tentative milestone of 1.1 for now

[bsclifton]

Updated the title also, since this is related to the context menu and not the fault of LastPass or our extension support

[diracdeltas]

Please ask me before pushing security issues back by more than 1 milestone. I think we need this fixed sooner since it is a cross-origin attack.

[bsclifton]

@diracdeltas will do- apologies

[bsclifton]

Moving to 0.15.0