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

Adding a custom subscription list with a nonexistent URL crashes on Android #31938

Closed
antonok-edm opened this issue Jul 27, 2023 · 2 comments · Fixed by brave/brave-core#19462
Assignees
Labels
crash dev-concern feature/shields/adblock Blocking ads & trackers with Shields OS/Android Fixes related to Android browser functionality OS/Desktop QA/No release-notes/exclude

Comments

@antonok-edm
Copy link
Contributor

@tapanmodh noticed that adding a custom adblock subscription list with a random URL would cause the browser to crash, but only on Android.

Turns out #28556 is back. We're using std::vector<T>::data() without a corresponding zero-length check.

@antonok-edm
Copy link
Contributor Author

Looks like we're missing some zero-length checks in a few other FFI functions too.

I am praying for the imminent merge of brave/brave-core#17368, but in the meantime brave/brave-core#19462 will have to do.

@kjozwiak
Copy link
Member

Verification PASSED on Pixel 6 running Android 14 using the following build(s):

Brave | 1.57.50 Chromium: 116.0.5845.96 (Official Build) (64-bit)
--- | ---
Revision | dd68ca599736ea1ec946e2e9def70f3c705895e1
OS | Android 14; Build/UPB5.230623.003; 34; REL

Using the STR/Cases outlined via,#32301 (comment), ensured that Brave wasn't crashing when adding https://antonok.com/tmp/empty.txt into brave://adblock. Restarted Brave several times and ensured there's no crashes/issues.

Screenshot_20230819-002913

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crash dev-concern feature/shields/adblock Blocking ads & trackers with Shields OS/Android Fixes related to Android browser functionality OS/Desktop QA/No release-notes/exclude
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants