HTTP x-frame-options: deny header makes the render process crash (77.1.12+gc63c001+chromium-77.0.3865.90)

[magreenblatt]

Original report by Masayuki Nagamachi (Bitbucket: Masayuki Nagamachi).

---

What steps will reproduce the problem?

1. Run cefsimple --url=https://erlend.oftedal.no/blog/tools/xframeoptions/

2. The render process crashes with the following messages:

   [1009/162452.386141:INFO:CONSOLE(0)] "Refused to display 'https://erlend.oftedal.no/blog/tools/xframeoptions/frame.php?img=no&header=deny' in a frame because it set 'X-Frame-Options' to 'deny'.", source: https://erlend.oftedal.no/blog/tools/xframeoptions/ (0) [1009/162452.406430:ERROR:bad_message.cc(27)] Terminating renderer for bad IPC message, reason 216

The “reason 216” is RFH_NO_MATCHING_NAVIGATION_REQUEST_ON_COMMIT which is defined in //content/browser/bad_message.h. This enum was introduced at 5902535cc6e6bbdfb4698493ea608845f77328a4.

The same issue occurs when loading https://www.youtube.com/.

What is the expected output? What do you see instead?

The render process shouldn’t crash even when receiving HTTP x-frame-options: deny header.

Similar messages are shown when loading the URL with Chrome 77.0.3865.90 (Official Build) (64-bit) for Linux/Desktop. But the render process never crashes.

What version of the product are you using? On what operating system?

Found this issue when running cefsimple downloaded from http://opensource.spotify.com/cefbuilds/cef_binary_77.1.12%2Bgc63c001%2Bchromium-77.0.3865.90_linux64_client.tar.bz2.

But this issue may also occur in older versions.

‌

[magreenblatt]

Original changes by Masayuki Nagamachi (Bitbucket: Masayuki Nagamachi).

---

• changed component from "Unclassified" to "Framework"

[magreenblatt]

Original changes by Masayuki Nagamachi (Bitbucket: Masayuki Nagamachi).

---

• edited description
• changed title from "HTTP x-frame-options: deny header makes cefsimple crash (77.1.12+gc63c001+chromium-77.0.3865.90)" to "HTTP x-frame-options: deny header makes the render process crash (77.1.12+gc63c001+chromium-77.0.3865.90)"

[magreenblatt]

Original changes by Masayuki Nagamachi (Bitbucket: Masayuki Nagamachi).

---

• edited description

[magreenblatt]

Interesting, the “bad IPC message” error occurs when loading the URL in cefsimple but not cefclient in my testing on Windows 10. Here’s the call stack leading to LogBadMessage in 77.0.3865.90:

>   content.dll!content::bad_message::`anonymous namespace'::LogBadMessage(content::bad_message::BadMessageReason reason) Line 27   C++
    content.dll!content::bad_message::ReceivedBadMessage(content::RenderProcessHost * host, content::bad_message::BadMessageReason reason) Line 49  C++
    content.dll!content::RenderFrameHostImpl::DidCommitNavigationInternal(std::__1::unique_ptr<content::NavigationRequest,std::__1::default_delete<content::NavigationRequest>> navigation_request, FrameHostMsg_DidCommitProvisionalLoad_Params * validated_params, bool is_same_document_navigation) Line 6830    C++
    content.dll!content::RenderFrameHostImpl::DidCommitNavigation(std::__1::unique_ptr<content::NavigationRequest,std::__1::default_delete<content::NavigationRequest>> committing_navigation_request, std::__1::unique_ptr<FrameHostMsg_DidCommitProvisionalLoad_Params,std::__1::default_delete<FrameHostMsg_DidCommitProvisionalLoad_Params>> validated_params, mojo::StructPtr<content::mojom::DidCommitProvisionalLoadInterfaceParams> interface_params) Line 7200 C++
    content.dll!content::RenderFrameHostImpl::DidCommitProvisionalLoad(std::__1::unique_ptr<FrameHostMsg_DidCommitProvisionalLoad_Params,std::__1::default_delete<FrameHostMsg_DidCommitProvisionalLoad_Params>> validated_params, mojo::StructPtr<content::mojom::DidCommitProvisionalLoadInterfaceParams> interface_params) Line 2383 C++
    content.dll!content::mojom::FrameHostStubDispatch::Accept(content::mojom::FrameHost * impl, mojo::Message * message) Line 5980  C++
    content.dll!content::mojom::FrameHostStub<mojo::RawPtrImplRefTraits<content::mojom::FrameHost>>::Accept(mojo::Message * message) Line 836   C++
    bindings.dll!mojo::InterfaceEndpointClient::HandleValidatedMessage(mojo::Message * message) Line 554    C++
    bindings.dll!mojo::InterfaceEndpointClient::HandleIncomingMessageThunk::Accept(mojo::Message * message) Line 140    C++
    bindings.dll!mojo::FilterChain::Accept(mojo::Message * message) Line 40 C++

‌

[magreenblatt]

Original comment by Masayuki Nagamachi (Bitbucket: Masayuki Nagamachi).

---

Confirmed that removing the following line can solve the crash issue:
https://bitbucket.org/chromiumembedded/cef/src/c63c0011422982f0388293b7262242210afff83e/tests/cefsimple/simple_handler.cc#lines-115

The content module commits a new navigation request in order to show an error page:

#2 0x7f333ac3dc3c content::RenderFrameImpl::DidCommitNavigationInternal()
#3 0x7f333ac3d63e content::RenderFrameImpl::DidCommitProvisionalLoad()
#4 0x7f3339c932bc blink::LocalFrameClientImpl::DispatchDidCommitLoad()
#5 0x7f333a1c62b3 blink::DocumentLoader::DidCommitNavigation()
#6 0x7f333a1c3002 blink::DocumentLoader::InstallNewDocument()
#7 0x7f333a1c28d4 blink::DocumentLoader::FinishNavigationCommit()
#8 0x7f333a1c59cf blink::DocumentLoader::StartLoadingResponse()
#9 0x7f333a1d3374 blink::FrameLoader::CommitNavigation()
#10 0x7f3339d86dc4 blink::WebLocalFrameImpl::CommitNavigation()
#11 0x7f333ac36052 content::RenderFrameImpl::CommitFailedNavigationInternal()
#12 0x7f333ac35461 content::RenderFrameImpl::CommitFailedNavigation()
#13 0x7f3334e52671 content::mojom::FrameNavigationControlStubDispatch::AcceptWithResponder()
#14 0x7f333acbe926 content::mojom::FrameNavigationControlStub<>::AcceptWithResponder()

In addition, SimpleHandler::OnLoadError() tries to load an error HTML string into the frame when failed to load a URL:

#2 0x7f333ac3dc3c content::RenderFrameImpl::DidCommitNavigationInternal()
#3 0x7f333ac3d63e content::RenderFrameImpl::DidCommitProvisionalLoad()
#4 0x7f3339c932bc blink::LocalFrameClientImpl::DispatchDidCommitLoad()
#5 0x7f333a1c62b3 blink::DocumentLoader::DidCommitNavigation()
#6 0x7f333a1c3002 blink::DocumentLoader::InstallNewDocument()
#7 0x7f333a1c2917 blink::DocumentLoader::FinishNavigationCommit()
#8 0x7f333a1c59cf blink::DocumentLoader::StartLoadingResponse()
#9 0x7f333a1d3374 blink::FrameLoader::CommitNavigation()
#10 0x7f3339d86dc4 blink::WebLocalFrameImpl::CommitNavigation()
#11 0x7f333ac5140a content::RenderFrameImpl::LoadHTMLString()
#12 0x7f3336a7aeb5 CefFrameImpl::OnRequest()
#13 0x7f3336a7aa70 IPC::MessageT<>::Dispatch<>()
#14 0x7f3336a7a820 CefFrameImpl::OnMessageReceived()
#15 0x7f333ac28b68 content::RenderFrameImpl::OnMessageReceived()
#16 0x7f33370dda61 IPC::ChannelProxy::Context::OnDispatchMessage()

As a result, RenderFrameHostImpl::DidCommitProvisionalLoad() is called twice.

navigation_request_ is moved in the first call of RenderFrameHostImpl::DidCommitProvisionalLoad(). So, navigation_request_ should be set before the next call. But it’s never set in the second call triggered by SimpleHandler::OnLoadError().

RenderFrameHostImpl::OnCrossDocumentCommitProcessed: 0x20b107003700  # navigation_request_ is set here
RenderFrameHostImpl::DidCommitProvisionalLoad: 0x20b107003700  # the first call by content::NavigationRequest::CommitErrorPage
RenderFrameHostImpl::DidCommitNavigationInternal
RenderFrameHostImpl::OnCrossDocumentCommitProcessed: 0x20b107001600
RenderFrameHostImpl::DidCommitProvisionalLoad: 0x20b107001600
RenderFrameHostImpl::DidCommitNavigationInternal:
RenderFrameHostImpl::DidCommitProvisionalLoad: 0x20b107003700  # the second call by SimpleHandler::OnLoadError
RenderFrameHostImpl::DidCommitNavigationInternal:
Terminating renderer for bad IPC message, reason 216

This crash issue can be simply solved by removing frame->LoadString(ss.str(), failedUrl) in SimpleHandler::OnLoadError(), or removing SimpleHandler::OnLoadError(). But CefFrame::LoadString() might have to be reworked.

[magreenblatt]

Original changes by Masayuki Nagamachi (Bitbucket: Masayuki Nagamachi).

---

• edited description

[magreenblatt]

Agreed, LoadString should not be called from OnLoadError. This looks like a variant of issue #2586.

[magreenblatt]

Duplicate of #2586.

[magreenblatt]

• changed state from "new" to "duplicate"

[magreenblatt]

Issue #2781 was marked as a duplicate of this issue.