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

verify our metascraper-based client code is not vulnerable #14066

Closed
diracdeltas opened this issue May 8, 2018 · 2 comments · Fixed by #14069
Closed

verify our metascraper-based client code is not vulnerable #14066

diracdeltas opened this issue May 8, 2018 · 2 comments · Fixed by #14069
Assignees
@diracdeltas @NejcZdovc
Labels
feature/rewards QA/checked-Linux QA/checked-macOS QA/checked-Win64 QA/test-plan-specified release-notes/exclude security
Milestone
0.22.x Release 3 ...

Comments

@diracdeltas
Copy link
Member

diracdeltas commented May 8, 2018 •
edited
Loading

the BAT client code in browser-laptop copy/pasted some code from metascraper, which has an unfixed vuln: https://hackerone.com/reports/309367. we need to check whether it is vulnerable as well.

related: #14065

UPDATE: no test plan is needed since we aren't vulnerable to the issue in the first place AFAICT, but it would be good for QA to go through the test plan in #13114 (comment) to make sure it hasn't regressed.
@diracdeltas diracdeltas added this to the 0.22.x Release 3 (Beta channel) milestone May 8, 2018
@diracdeltas
Copy link
Member Author

Pretty sure this is OK because we don't evaluate any of the metascraper input as code. I will add a check to sanitize it for HTML just to be extra sure.

diracdeltas added a commit that referenced this issue May 8, 2018
@diracdeltas
add comment about metascraper untrusted input, sanitize HTML
a93fea1 
fix #14066

Test Plan:
follow test plan in #13114
to make sure there are no regressions
diracdeltas added a commit that referenced this issue May 8, 2018
@diracdeltas
add comment about metascraper untrusted input, sanitize HTML
94b507c 
fix #14066

Test Plan:
follow test plan in #13114
to make sure there are no regressions
@diracdeltas diracdeltas mentioned this issue May 8, 2018
Merged
10 tasks
diracdeltas added a commit that referenced this issue May 9, 2018
@diracdeltas
add comment about metascraper untrusted input, sanitize HTML
a5fb6e9 
fix #14066

Test Plan:
follow test plan in #13114
to make sure there are no regressions
This was referenced May 10, 2018
Closed
Closed
Closed
@LaurenWags
Copy link
Member

LaurenWags commented May 10, 2018 •
edited by btlechowski
Loading

Verified test plan from #13114 (comment) with macOS 10.12.6 using

  • 0.22.706 e11b027
  • muon 6.0.9
  • libchromiumcontent 66.0.3359.139

Verified on Windows x64

  • 0.22.706 e11b027
  • libchromiumcontent 66.0.3359.139
  • muon: 6.0.9

Verified on Ubuntu 17.10 x64

  • 0.22.706 e11b027
  • libchromiumcontent 66.0.3359.139
  • muon: 6.0.9

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

@diracdeltas diracdeltas

@NejcZdovc NejcZdovc

Labels
feature/rewards QA/checked-Linux QA/checked-macOS QA/checked-Win64 QA/test-plan-specified release-notes/exclude security
Projects
None yet
Milestone
0.22.x Release 3 (Release Channel)
Development

Successfully merging a pull request may close this issue.

add comment about metascraper untrusted input, sanitize HTML brave/browser-laptop
6 participants
@diracdeltas @bsclifton @NejcZdovc @srirambv @LaurenWags @btlechowski