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

URL check in bat_get_media.cc is not strict enough #3462

Closed
diracdeltas opened this issue Feb 22, 2019 · 3 comments · Fixed by brave/brave-core#1792
Closed

URL check in bat_get_media.cc is not strict enough #3462

diracdeltas opened this issue Feb 22, 2019 · 3 comments · Fixed by brave/brave-core#1792

Comments

@diracdeltas
Copy link
Member

diracdeltas commented Feb 22, 2019

https://github.com/brave/brave-core/blob/194e5db6b091490576ce8bd191004cd12dc5c7d6/vendor/bat-native-ledger/src/bat_get_media.cc#L46

i assume url.find(".ttvnw.net/v1/segment/") is trying to match subdomains of ttvnw.net, but it would actually match arbitrary domains like this: https://example.com/something.ttvnw.net/v1/segment/ because . is valid in URL paths.

cc @SergeyZhukovsky @jasonrsadler

@NejcZdovc NejcZdovc added the priority/P2 A bad problem. We might uplift this to the next planned release. label Feb 23, 2019
@jasonrsadler
Copy link

We're going to need some kind of url parser for this.

@bridiver Can we use GURL for this?

https://github.com/brave/brave-core/blob/194e5db6b091490576ce8bd191004cd12dc5c7d6/vendor/bat-native-ledger/src/bat_get_media.cc#L47

url.find("https://ttvnw.net/v1/segment/") != std::string::npos would match: https://baddomain.com?maliciousparam=https://ttvnw.net/v1/segment/

@jasonrsadler
Copy link

Adding QA-Yes for regression testing

@btlechowski
Copy link

btlechowski commented Mar 20, 2019

Verification passed on

Brave 0.63.14 Chromium: 73.0.3683.75 (Official Build) dev (64-bit)
Revision 909ee014fcea6828f9a610e6716145bc0b3ebf4a-refs/branch-heads/3683@{#803}
OS Windows 10 OS Build 17134.523

Verified test plan from brave/brave-core#1792
Encountered #3590

Verification passed on

Brave 0.63.15 Chromium: 73.0.3683.75 (Official Build) dev (64-bit)
Revision 909ee014fcea6828f9a610e6716145bc0b3ebf4a-refs/branch-heads/3683@{#803}
OS Linux

Verified passed with

Brave 0.63.31 Chromium: 73.0.3683.75 (Official Build) beta(64-bit)
Revision 909ee014fcea6828f9a610e6716145bc0b3ebf4a-refs/branch-heads/3683@{#803}
OS Mac OS X

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

Successfully merging a pull request may close this issue.

7 participants