Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

cookie testing site shows leaks #14250

Closed
diracdeltas opened this issue May 24, 2018 · 6 comments · Fixed by #14286
Closed

cookie testing site shows leaks #14250

diracdeltas opened this issue May 24, 2018 · 6 comments · Fixed by #14286
Assignees
@Slava
Labels
priority/P3 Major loss of function. privacy QA/checked-Linux QA/checked-macOS QA/checked-Win64 QA/test-plan-specified release-notes/include
Milestone
0.23.x Release 5 ...

Comments

@diracdeltas
Copy link
Member

diracdeltas commented May 24, 2018 •
edited by bsclifton
Loading

Test plan

  1. Manually navigate to https://www.grc.com/cookies/forensics.htm and observe no problems in the report
  2. Navigate to a website with a favicon (such as github.com) and observe the favicon rendering

Original description

STR:

  1. go to https://www.grc.com/cookies/forensics.htm
  2. it shows "trouble" with our 3rd party cookie blocking
@bsclifton bsclifton added this to the Triage Backlog milestone May 25, 2018
@riastradh-brave
Copy link
Contributor

For comparison, the results turn up all green in the Tor Browser.

I haven't studied the details of what the page is doing.

@riastradh-brave
Copy link
Contributor

The results turn up red for third-party cookies in Brave no matter what my cookie settings in shields are: block all, allow 3rd party, allow all.

@tildelowengrimm tildelowengrimm added needs-investigation A bug not 100% confirmed/fixed that needs QA to better audit. priority/P3 Major loss of function. fixed-with-brave-core This issue will automatically resolved with the replacement of Muon with Brave Core. labels May 29, 2018
Slava added a commit to Slava/browser-laptop that referenced this issue May 30, 2018
@Slava
Strip cookies from requests fetch favicons
41e3ffd 
Fixes brave#14250
Auditors: @diracdeltas

Test Plan:
Manually navigate to  https://www.grc.com/cookies/forensics.htm and observe no problems in the report
Navigate to a website with a favicon (such as github.com) and observe the favicon rendering
@Slava Slava mentioned this issue May 30, 2018
Merged
10 tasks
Slava added a commit to Slava/browser-laptop that referenced this issue May 31, 2018
@Slava
Strip cookies from requests fetch favicons
cf38a06 
Fixes brave#14250
Auditors: @diracdeltas

Test Plan:
Manually navigate to  https://www.grc.com/cookies/forensics.htm and observe no problems in the report
Navigate to a website with a favicon (such as github.com) and observe the favicon rendering

Unit tests:
run the faviconUtil unit tests

Webdriver test:
Runs a test page that sets a 3rd party cookie and checks if the favicon request carries over the cookies
@Slava
Copy link
Contributor

Slava commented May 31, 2018

I made a PR to address this. FWIW the page linked is a bit flaky for me on Safari and consistently fails in Chrome in incognito. I don't understand how the page is running the tests exactly but with the change it should pass on Brave consistently.

Slava added a commit to Slava/browser-laptop that referenced this issue May 31, 2018
@Slava
Strip cookies from requests fetch favicons
800028f 
Fixes brave#14250
Auditors: @diracdeltas

Test Plan:
Manually navigate to  https://www.grc.com/cookies/forensics.htm and observe no problems in the report
Navigate to a website with a favicon (such as github.com) and observe the favicon rendering

Unit tests:
run the faviconUtil unit tests

Webdriver test:
Runs a test page that sets a 3rd party cookie and checks if the favicon request carries over the cookies
@bsclifton
Copy link
Member

Uplifted to 0.23.x with e104f5a

@kjozwiak
Copy link
Member

@bsclifton removing the fixed-with-brave-core tag as it appears as this has been fixed and is slated to be released within the 0.23.x Release 5.

@kjozwiak kjozwiak removed fixed-with-brave-core This issue will automatically resolved with the replacement of Muon with Brave Core. needs-investigation A bug not 100% confirmed/fixed that needs QA to better audit. labels Aug 14, 2018
This was referenced Aug 14, 2018
Closed
Closed
Closed
@LaurenWags
Copy link
Member

LaurenWags commented Aug 14, 2018 •
edited by kjozwiak
Loading

Verified with macOS 10.12.6 using

  • 0.23.80 53a429f
  • Muon 8.0.8
  • libchromiumcontent 68.0.3440.84

Verified on Ubuntu 18 x64 using

  • 0.23.80 53a429f
  • Muon 8.0.8
  • libchromiumcontent 68.0.3440.84

Verified on Windows x64 with

  • 0.23.80 53a429f
  • Muon 8.0.8
  • libchromiumcontent 68.0.3440.84

@kjozwiak kjozwiak mentioned this issue Aug 28, 2018
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@Slava Slava

Labels
priority/P3 Major loss of function. privacy QA/checked-Linux QA/checked-macOS QA/checked-Win64 QA/test-plan-specified release-notes/include
Projects
None yet
Milestone
0.23.x Release 5 (Release Channel)
Development

Successfully merging a pull request may close this issue.

Strip cookies from requests fetch favicons Slava/browser-laptop
9 participants
@diracdeltas @tildelowengrimm @kjozwiak @Slava @bsclifton @srirambv @LaurenWags @riastradh-brave @GeetaSarvadnya