Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unsafe memory write in BraveNetworkDelegateBase #3176

Closed
iefremov opened this issue Jan 31, 2019 · 2 comments · Fixed by brave/brave-core#1531
Closed

Unsafe memory write in BraveNetworkDelegateBase #3176

iefremov opened this issue Jan 31, 2019 · 2 comments · Fixed by brave/brave-core#1531

Comments

@iefremov
Copy link
Contributor

Description

In BraveNetworkDelegateBase::GetReferralHeaders()[1] pointer is written on UI thread, at the some moment it could be read on IO thread.

[1] https://github.com/brave/brave-core/blob/master/browser/net/brave_network_delegate_base.cc#L86

Brave version (brave://version info)

Brave 0.62.0 Chromium: 72.0.3626.71 (Developer Build) (64-bit)
Revision f52ccad2a6a3c65fc9e0c591a517ceab1198dac0-refs/branch-heads/3626@{#763}

Reproducible on current release:

yes

@iefremov iefremov self-assigned this Jan 31, 2019
@iefremov iefremov added this to the 0.62.x - Nightly milestone Jan 31, 2019
@rebron rebron added the QA/Yes label Feb 1, 2019
@rebron
Copy link
Collaborator

rebron commented Feb 1, 2019

cc: @brave/legacy_qa please see pr for details

@btlechowski
Copy link

btlechowski commented Mar 13, 2019

Verification passed on

Brave 0.62.25 Chromium: 73.0.3683.67 (Official Build) beta (64-bit)
Revision a83fd4f3207ae83412d329a9ca1239dd1e068345-refs/branch-heads/3683@{#760}
OS Windows 7 Service Pack 1 Build 7601.24312

Test plan is based on this comment: brave/brave-core#1531 (comment)
Verified marketwatch.com and barrons.com got X-Brave-Partner: dowjones header
Verified cheddar.com got X-Brave-Partner: cheddar header
Verified coinbase.com got x-brave-partner: coinbase header

image
image

Verification PASSED on macOS 10.14.3 x64 using the following build:

Brave 0.62.27 Chromium: 73.0.3683.75 (Official Build) beta(64-bit)
Revision 909ee014fcea6828f9a610e6716145bc0b3ebf4a-refs/branch-heads/3683@{#803}
OS Mac OS X
  • Went through the test plan mentioned under Fix concurrency brave-core#1531 (comment) and ensured the following:
    • Verified marketwatch.com and barrons.com displayed the X-Brave-Partner: dowjones header
    • Verified cheddar.com received the X-Brave-Partner: cheddar header
    • Verified coinbase.com received the x-brave-partner: coinbase header

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment