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

Web Discovery IsHashLikely crash #44653

Closed
DJAndries opened this issue Mar 13, 2025 · 2 comments · Fixed by brave/brave-core#28135
Closed

Web Discovery IsHashLikely crash #44653

DJAndries opened this issue Mar 13, 2025 · 2 comments · Fixed by brave/brave-core#28135

Comments

@DJAndries
Copy link
Collaborator

DJAndries commented Mar 13, 2025

Identified the following crash for Android Web Discovery users.

The crash happens here: https://github.com/brave/brave-core/blob/60597b34e7a77b61678754bdaac83b0594b05b74/components/web_discovery/browser/hash_detection.cc#L46

It seems to occur when the search query ends with a non-alphanumeric character. The loop before the mentioned line iterates though all characters, leading to illegal access.

[ 00 ] web_discovery::IsHashLikely(std::__Cr::basic_string_view<char, std::__Cr::char_traits<char>>, double) ( hash_detection.cc:0 )
[ 01 ] web_discovery::IsPrivateQueryLikely(std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&) ( privacy_guard.cc:162 )
[ 02 ] web_discovery::WebDiscoveryService::OnContentScraped(bool, std::__Cr::unique_ptr<web_discovery::PageScrapeResult, std::__Cr::default_delete<web_discovery::PageScrapeResult>>) ( web_discovery_service.cc:218 )
[ 03 ] web_discovery::(anonymous namespace)::ContentScraperImpl::OnRendererScrapedElementAttributes(bool, std::__Cr::unique_ptr<web_discovery::PageScrapeResult, std::__Cr::default_delete<web_discovery::PageScrapeResult>>, base::OnceCallback<void (std::__Cr::unique_ptr<web_discovery::PageScrapeResult, std::__Cr::default_delete<web_discovery::PageScrapeResult>>)>, std::__Cr::vector<mojo::StructPtr<web_discovery::mojom::AttributeResult>, std::__Cr::allocator<mojo::StructPtr<web_discovery::mojom::AttributeResult>>>) ( callback.h:156 )
[ 04 ] base::internal::Invoker<base::internal::FunctorTraits<void (web_discovery::(anonymous namespace)::ContentScraperImpl::*&&)(bool, std::__Cr::unique_ptr<web_discovery::PageScrapeResult, std::__Cr::default_delete<web_discovery::PageScrapeResult>>, base::OnceCallback<void (std::__Cr::unique_ptr<web_discovery::PageScrapeResult, std::__Cr::default_delete<web_discovery::PageScrapeResult>>)>, std::__Cr::vector<mojo::StructPtr<web_discovery::mojom::AttributeResult>, std::__Cr::allocator<mojo::StructPtr<web_discovery::mojom::AttributeResult>>>), web_discovery::(anonymous namespace)::ContentScraperImpl*, bool&&, std::__Cr::unique_ptr<web_discovery::PageScrapeResult, std::__Cr::default_delete<web_discovery::PageScrapeResult>>&&, base::OnceCallback<void (std::__Cr::unique_ptr<web_discovery::PageScrapeResult, std::__Cr::default_delete<web_discovery::PageScrapeResult>>)>&&>, base::internal::BindState<true, true, false, void (web_discovery::(anonymous namespace)::ContentScraperImpl::*)(bool, std::__Cr::unique_ptr<web_discovery::PageScrapeResult, std::__Cr::default_delete<web_discovery::PageScrapeResult>>, base::OnceCallback<void (std::__Cr::unique_ptr<web_discovery::PageScrapeResult, std::__Cr::default_delete<web_discovery::PageScrapeResult>>)>, std::__Cr::vector<mojo::StructPtr<web_discovery::mojom::AttributeResult>, std::__Cr::allocator<mojo::StructPtr<web_discovery::mojom::AttributeResult>>>), base::internal::UnretainedWrapper<web_discovery::(anonymous namespace)::ContentScraperImpl, base::unretained_traits::MayNotDangle, (partition_alloc::internal::RawPtrTraits)0>, bool, std::__Cr::unique_ptr<web_discovery::PageScrapeResult, std::__Cr::default_delete<web_discovery::PageScrapeResult>>, base::OnceCallback<void (std::__Cr::unique_ptr<web_discovery::PageScrapeResult, std::__Cr::default_delete<web_discovery::PageScrapeResult>>)>>, void (std::__Cr::vector<mojo::StructPtr<web_discovery::mojom::AttributeResult>, std::__Cr::allocator<mojo::StructPtr<web_discovery::mojom::AttributeResult>>>)>::RunOnce(base::internal::BindStateBase*, std::__Cr::vector<mojo::StructPtr<web_discovery::mojom::AttributeResult>, std::__Cr::allocator<mojo::StructPtr<web_discovery::mojom::AttributeResult>>>&&) ( bind_internal.h:728 )
[ 05 ] web_discovery::mojom::DocumentExtractor_QueryElementAttributes_ForwardToCallback::Accept(mojo::Message*) ( callback.h:156 )
[ 06 ] mojo::InterfaceEndpointClient::HandleValidatedMessage(mojo::Message*) ( interface_endpoint_client.cc:1046 )
[ 07 ] mojo::InterfaceEndpointClient::HandleIncomingMessage(mojo::Message*) ( interface_endpoint_client.cc:371 )
[ 08 ] mojo::internal::MultiplexRouter::Accept(mojo::Message*) ( multiplex_router.cc:1121 )
[ 09 ] mojo::MessageDispatcher::Accept(mojo::Message*) ( message_dispatcher.cc:43 )
[ 10 ] base::internal::Invoker<base::internal::FunctorTraits<void (mojo::Connector::* const&)(char const*, unsigned int), mojo::Connector*, char const* const&>, base::internal::BindState<true, true, false, void (mojo::Connector::*)(char const*, unsigned int), base::internal::UnretainedWrapper<mojo::Connector, base::unretained_traits::MayNotDangle, (partition_alloc::internal::RawPtrTraits)0>, base::internal::UnretainedWrapper<char const, base::unretained_traits::MayNotDangle, (partition_alloc::internal::RawPtrTraits)0>>, void (unsigned int)>::Run(base::internal::BindStateBase*, unsigned int) ( connector.cc:562 )
[ 11 ] base::internal::Invoker<base::internal::FunctorTraits<void (mojo::SimpleWatcher::*&&)(int, unsigned int, mojo::HandleSignalsState const&), base::WeakPtr<mojo::SimpleWatcher>&&, int&&, unsigned int&&, mojo::HandleSignalsState&&>, base::internal::BindState<true, true, false, void (mojo::SimpleWatcher::*)(int, unsigned int, mojo::HandleSignalsState const&), base::WeakPtr<mojo::SimpleWatcher>, int, unsigned int, mojo::HandleSignalsState>, void ()>::RunOnce(base::internal::BindStateBase*) ( callback.h:344 )
[ 12 ] non-virtual thunk to base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork() ( callback.h:156 )
[ 13 ] base::MessagePumpAndroid::DoNonDelayedLooperWork(bool) ( message_pump_android.cc:456 )
[ 14 ] base::(anonymous namespace)::NonDelayedLooperCallback(int, int, void*) ( message_pump_android.cc:441 )
[ 15 ] 0x76f745d224
[ 16 ] 0x76f745d224
[ 17 ] 0x76f745cda0
[ 18 ] 0x76e79ddacc
[ 19 ] 0x71d7d098
[ 20 ] 0x71d79ba0
[ 21 ] 0x71d79a98
[ 22 ] 0x71ac1a0c
[ 23 ] 0x7660a0e17c
[ 24 ] 0x7660168a40
[ 25 ] 0x7660a0e17c
[ 26 ] 0x76601644d4
[ 27 ] 0x76e7940564
[ 28 ] 0x716a902c
[ 29 ] 0x716a9c80
[ 30 ] 0x71cf9edc
[ 31 ] 0x71cfbd2c
[ 32 ] 0x76604c9298
[ 33 ] 0x716aea04
[ 34 ] 0x71cf4848
[ 35 ] 0x71cfe860
[ 36 ] 0x7660168a40
[ 37 ] 0x7660153f4c
[ 38 ] 0x7660151f00
[ 39 ] 0x766053da58
[ 40 ] 0x76e7aa9ffc
[ 41 ] 0x76e7aaacc4
[ 42 ] 0x7660a0cffc
[ 43 ] 0x76e7937ce8
[ 44 ] 0x76e7aaacc4
[ 45 ] JniInvocationDestroy
[ 46 ] 0x76e78f60ab
[ 47 ] 0x76e7944444
[ 48 ] 0x576e6fb36f
[ 49 ] 0x76f0b6f5c4
[ 50 ] 0x770a6127fc
[ 51 ] 0x76f0ab93a4
[ 52 ] 0x576e6fb39f
[ 53 ] 0x76f0b6f5c4
[ 54 ] 0x770a6127fc
[ 55 ] 0x76f0ab90b4
[ 56 ] 0x76f7459e98
[ 57 ] 0x76e7940564
[ 58 ] 0x576e6fb39f
[ 59 ] 0x576e6fc574
[ 60 ] 0x576e6fb39f
[ 61 ] 0x576e6fd07c
[ 62 ] 0x576e6fd07c
[ 63 ] 0x576e6fd03c
[ 64 ] 0x76e7a8e1b4
[ 65 ] 0x576e6fd07c
@kjozwiak
Copy link
Member

The above requires 1.77.82 or higher for 1.77.x verification 👍 @Uni-verse @hffvld can use brave/brave-core#28135 (comment) as a template for the 1.77.x verification.

@hffvld hffvld added the QA/In-Progress Indicates that QA is currently in progress for that particular issue label Mar 17, 2025
@hffvld
Copy link
Contributor

hffvld commented Mar 17, 2025

Verified on Galaxy Tab S8 and Pixel 7 using version(s):

Device/OS: 
- Galaxy Tab S8 / gts8wifixx-user 14 UP1A.231005.007 release-keys
- Pixel 7 / panther_beta-user 16 BP22.250103.008 release-keys
Brave build: 1.77.82 
Chromium: 134.0.6998.95 (Official Build) beta (64-bit) 

STEPS:

  1. Follow the STR/TP from Fix Web Discovery IsHashLikely crash brave-core#28135 (comment) and Fix Web Discovery IsHashLikely crash brave-core#28135 (comment)
  2. Verify

ACTUAL RESULTS:

  • Verified that request is made to https://patterns.wdp.brave.com/patterns-android.gz.
  • Verified that Brave doesn't crash when sending a Google query ending with a non-alphanumeric character like the very quick fox!.

Galaxy Tab S8

1 2 3 4
1 2 3 4

Pixel 7

1 2 3 4
1 2 3 4

@hffvld hffvld added QA Pass - Android ARM QA Pass - Android Tab and removed QA/In-Progress Indicates that QA is currently in progress for that particular issue labels Mar 17, 2025
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.

4 participants