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

[hackerone] Crash when loading brave://optimization-guide-internals/ #31648

Closed
fmarier opened this issue Jul 14, 2023 · 7 comments · Fixed by brave/brave-core#19984
Closed

[hackerone] Crash when loading brave://optimization-guide-internals/ #31648

fmarier opened this issue Jul 14, 2023 · 7 comments · Fixed by brave/brave-core#19984
Assignees
Labels
Chromium/reported upstream Issue has been reported upstream and crbug link is in the issue crash OS/Android Fixes related to Android browser functionality OS/Desktop priority/P2 A bad problem. We might uplift this to the next planned release. QA Pass - Android ARM QA Pass - Android 8+ QA Pass-macOS QA/Yes release-notes/include security

Comments

@fmarier
Copy link
Member

fmarier commented Jul 14, 2023

Brave Version: 1.57.1 Chromium: 115.0.5790.56
Operating System: Linux 6.2.6-76060206-generic

URL (if applicable) where crash occurred: brave://optimization-guide-internals/

Can you reproduce this crash? Yes

What steps will reproduce this crash? (If it's not reproducible, what were you doing just before the crash?)

  1. Open new tab.
  2. Type brave://optimization-guide-internals/ in the URL bar.
  3. Press Enter

DO NOT CHANGE BELOW THIS LINE
Crash ID: crash/13670300-e8f6-640a-0000-000000000000

@fmarier fmarier added the crash label Jul 14, 2023
@fmarier
Copy link
Member Author

fmarier commented Jul 14, 2023

Originally reported at https://hackerone.com/reports/2069544

@fmarier fmarier added security OS/Android Fixes related to Android browser functionality OS/Desktop labels Jul 14, 2023
@rebron rebron added the priority/P2 A bad problem. We might uplift this to the next planned release. label Jul 14, 2023
@mkarolin
Copy link
Contributor

This is an upstream bug that exhibits itself because we disable kOptimizationHints feature. When creating this WebUI controller Chromium checks for the feature being enabled and if not returns nullptr https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/optimization_guide/optimization_guide_keyed_service_factory.cc;l=19. However, the NavigationRequest only has a DCHECK if the returned controller is nullptr (https://source.chromium.org/chromium/chromium/src/+/main:content/browser/renderer_host/navigation_request.cc;l=9586 - creates controller, and later DCHECK in WebUIImpl::SetController).

STR with Chrome:

  1. Start Chrome with "C:\Program Files (x86)\Google\Chrome Beta\Application\chrome.exe" --disable-features=OptimizationHints
  2. Navigate to brave://optimization-guide-internals/
  3. Observe crash.

@rebron
Copy link
Collaborator

rebron commented Aug 25, 2023

cc: @simonhong, @iefremov

@mkarolin
Copy link
Contributor

Filed upstream https://bugs.chromium.org/p/chromium/issues/detail?id=1476101

@simonhong simonhong self-assigned this Aug 25, 2023
@rebron rebron added the Chromium/reported upstream Issue has been reported upstream and crbug link is in the issue label Aug 30, 2023
simonhong added a commit to brave/brave-core that referenced this issue Sep 5, 2023
fix brave/brave-browser#31648

WebUI instance should not be created for that url when optimization
hint is disabled. It's upstream issue and reported.
@brave-builds brave-builds added this to the 1.59.x - Nightly milestone Sep 5, 2023
@rebron rebron changed the title Crash Report for brave://optimization-guide-internals/ Crash when loading brave://optimization-guide-internals/ Sep 6, 2023
@bsclifton
Copy link
Member

Nice work on this @simonhong 🎉

@stephendonner
Copy link

Verified PASSED using

Brave | 1.59.79 Chromium: 117.0.5938.48 (Official Build) beta (x86_64)
-- | --
Revision | b93ab5e51eb1080a6242fea103da637d542ca312
OS | macOS Version 14.0 (Build 23A5337a)

First, reproduced the crash:

[ 00 ] content::WebUIImpl::WebUIRenderFrameCreated(content::RenderFrameHost*) ( web_ui_impl.cc:142 )
[ 01 ] content::RenderFrameHostManager::GetFrameHostForNavigation(content::NavigationRequest*, content::BrowsingContextGroupSwap*, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>>*) ( render_frame_host_manager.cc:1627 )
[ 02 ] content::RenderFrameHostManager::DidCreateNavigationRequest(content::NavigationRequest*) ( render_frame_host_manager.cc:1264 )
[ 03 ] content::FrameTreeNode::TakeNavigationRequest(std::__Cr::unique_ptr<content::NavigationRequest, std::__Cr::default_delete<content::NavigationRequest>>) ( frame_tree_node.cc:600 )
[ 04 ] content::Navigator::Navigate(std::__Cr::unique_ptr<content::NavigationRequest, std::__Cr::default_delete<content::NavigationRequest>>, content::ReloadType) ( navigator.cc:755 )
[ 05 ] content::NavigationControllerImpl::NavigateWithoutEntry(content::NavigationController::LoadURLParams const&) ( navigation_controller_impl.cc:3681 )
[ 06 ] content::NavigationControllerImpl::LoadURLWithParams(content::NavigationController::LoadURLParams const&) ( navigation_controller_impl.cc:1314 )
[ 07 ] (anonymous namespace)::LoadURLInContents(content::WebContents*, GURL const&, NavigateParams*) ( browser_navigator.cc:484 )
[ 08 ] Navigate(NavigateParams*) ( browser_navigator.cc:791 )
[ 09 ] chrome::OpenCurrentURL(Browser*) ( browser_commands.cc:784 )
[ 10 ] ChromeOmniboxClient::OnAutocompleteAccept(GURL const&, std::__Cr::pair<std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>>, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>>>*, WindowOpenDisposition, ui::PageTransition, AutocompleteMatchType::Type, base::TimeTicks, bool, bool, std::__Cr::basic_string<char16_t, std::__Cr::char_traits<char16_t>, std::__Cr::allocator<char16_t>> const&, AutocompleteMatch const&, AutocompleteMatch const&, IDNA2008DeviationCharacter) ( chrome_omnibox_client.cc:479 )
[ 11 ] OmniboxEditModel::OpenMatch(OmniboxPopupSelection, AutocompleteMatch, WindowOpenDisposition, GURL const&, std::__Cr::basic_string<char16_t, std::__Cr::char_traits<char16_t>, std::__Cr::allocator<char16_t>> const&, base::TimeTicks) ( omnibox_edit_model.cc:2526 )
[ 12 ] OmniboxEditModel::OpenSelection(OmniboxPopupSelection, base::TimeTicks, WindowOpenDisposition) ( omnibox_edit_model.cc:894 )
[ 13 ] OmniboxViewViews::HandleKeyEvent(views::Textfield*, ui::KeyEvent const&) ( omnibox_view_views.cc:0 )
[ 14 ] views::Textfield::OnKeyPressed(ui::KeyEvent const&) ( textfield.cc:735 )
[ 15 ] views::View::OnKeyEvent(ui::KeyEvent*) ( view.cc:1515 )
[ 16 ] non-virtual thunk to views::View::OnKeyEvent(ui::KeyEvent*) ( view.cc:0 )
[ 17 ] ui::EventDispatcher::DispatchEvent(ui::EventHandler*, ui::Event*) ( event_dispatcher.cc:187 )
[ 18 ] ui::EventDispatcher::ProcessEvent(ui::EventTarget*, ui::Event*) ( event_dispatcher.cc:136 )
[ 19 ] ui::EventDispatcherDelegate::DispatchEventToTarget(ui::EventTarget*, ui::Event*) ( event_dispatcher.cc:82 )
[ 20 ] ui::EventDispatcherDelegate::DispatchEvent(ui::EventTarget*, ui::Event*) ( event_dispatcher.cc:54 )
[ 21 ] ui::EventProcessor::OnEventFromSource(ui::Event*) ( event_processor.cc:72 )
[ 22 ] ui::EventSource::SendEventToSinkFromRewriter(ui::Event const*, ui::EventRewriter const*) ( event_source.cc:143 )
[ 23 ] views::Widget::OnKeyEvent(ui::KeyEvent*) ( widget.cc:1699 )
[ 24 ] views::NativeWidgetMac::DispatchKeyEventPostIME(ui::KeyEvent*) ( native_widget_mac.mm:978 )
[ 25 ] non-virtual thunk to views::NativeWidgetMac::DispatchKeyEventPostIME(ui::KeyEvent*) ( native_widget_mac.h:0 )
[ 26 ] ui::InputMethodBase::DispatchKeyEventPostIME(ui::KeyEvent*) const ( input_method_base.cc:139 )
[ 27 ] -[BridgedContentView handleUnhandledKeyDownAsKeyEvent] ( bridged_content_view.mm:438 )
[ 28 ] -[BridgedContentView keyDown:] ( bridged_content_view.mm:835 )
[ 29 ] 0x7ff80965abfe
[ 30 ] 0x7ff80965a7db
[ 31 ] -[NativeWidgetMacNSWindow sendEvent:] ( native_widget_mac_nswindow.mm:473 )
[ 32 ] 0x7ff809dea576
[ 33 ] __34-[BrowserCrApplication sendEvent:]_block_invoke ( chrome_browser_application_mac.mm:354 )
[ 34 ] base::mac::CallWithEHFrame(void () block_pointer)
[ 35 ] -[BrowserCrApplication sendEvent:] ( chrome_browser_application_mac.mm:330 )
[ 36 ] 0x7ff8099ac737
[ 37 ] 0x7ff8094ec83f
[ 38 ] base::MessagePumpNSApplication::DoRun(base::MessagePump::Delegate*) ( message_pump_mac.mm:811 )
[ 39 ] base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*) ( message_pump_mac.mm:167 )
[ 40 ] base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run(bool, base::TimeDelta) ( thread_controller_with_message_pump_impl.cc:651 )
[ 41 ] non-virtual thunk to base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run(bool, base::TimeDelta) ( thread_controller_with_message_pump_impl.cc:0 )
[ 42 ] base::RunLoop::Run(base::Location const&) ( run_loop.cc:134 )
[ 43 ] content::BrowserMainLoop::RunMainMessageLoop() ( browser_main_loop.cc:1067 )
[ 44 ] content::BrowserMainRunnerImpl::Run() ( browser_main_runner_impl.cc:159 )
[ 45 ] content::BrowserMain(content::MainFunctionParams) ( browser_main.cc:34 )
[ 46 ] content::RunBrowserProcessMain(content::MainFunctionParams, content::ContentMainDelegate*) ( content_main_runner_impl.cc:685 )
[ 47 ] content::ContentMainRunnerImpl::RunBrowser(content::MainFunctionParams, bool) ( content_main_runner_impl.cc:1266 )
[ 48 ] content::ContentMainRunnerImpl::Run() ( content_main_runner_impl.cc:1116 )
[ 49 ] content::RunContentProcess(content::ContentMainParams, content::ContentMainRunner*) ( content_main.cc:326 )
[ 50 ] content::ContentMain(content::ContentMainParams) ( content_main.cc:343 )
[ 51 ] ChromeMain ( chrome_main.cc:187 )
[ 52 ] main ( chrome_exe_main_mac.cc:216 )
[ 53 ] 0x7ff805a933a6

Then, confirmed the fix:

  1. installed 1.59.79
  2. loaded brave://optimization-guide-internals/

Confirmed no crash when loading the above URL

Screenshot 2023-09-08 at 3 30 07 PM

@LaurenWags LaurenWags changed the title Crash when loading brave://optimization-guide-internals/ [hackerone] Crash when loading brave://optimization-guide-internals/ Sep 15, 2023
@hffvld hffvld added the QA/In-Progress Indicates that QA is currently in progress for that particular issue label Sep 21, 2023
@hffvld
Copy link
Contributor

hffvld commented Sep 21, 2023

Verified on Pixel 2 XL and Pixel 7 using version(s):

Device/OS: 
- Pixel 2 XL [taimen-user  8.1.0 OPM2.171026.006.H1 release-keys]
- Pixel 7 [panther_beta-user 14 UPB5.230623.009 release-keys]
Brave build: 1.59.93
Chromium: 117.0.5938.88 (Official Build) beta (64-bit) 
Revision: 77474ed80832bd2c90be4ebf993b6c2a43f662b0

STEPS:

  1. Launch Brave
  2. NTP > Enter brave://optimization-guide-internals/ in the URL search bar
  3. Tap Enter > Verify

ACTUAL RESULTS:

  • Verified that Brave is not crashing when opening brave://optimization-guide-internals/

Pixel 2 XL / Android 8

timestamp_16-17-05_16-17-31.mp4

Pixel 7 / Android 14

timestamp_16-22-52_16-23-15.mp4

@hffvld hffvld added QA Pass - Android ARM QA Pass - Android 8+ and removed QA/In-Progress Indicates that QA is currently in progress for that particular issue labels Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Chromium/reported upstream Issue has been reported upstream and crbug link is in the issue crash OS/Android Fixes related to Android browser functionality OS/Desktop priority/P2 A bad problem. We might uplift this to the next planned release. QA Pass - Android ARM QA Pass - Android 8+ QA Pass-macOS QA/Yes release-notes/include security
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

8 participants