Re: [chromium-security] Is it safe to send the navigation origin to the renderer at ReadyToCommitNavigation time?

[Daniel Cheng]

I think it's fine to send the full URL. If the renderer were compromised, well, we're about to commit the navigation in it and allow it to read the entire contents of the URL anyway.

Of course, if we don't need the full URL, then sending the origin is better.

Daniel

On Fri, 5 Sept 2025 at 11:14, 'Alex Gough' via security <secu...@chromium.org> wrote:

Does https://chromium.googlesource.com/chromium/src/+/main/docs/security/mojo.md#do-not-send-unnecessary-or-privilege_presuming-data help?

On Fri, Sep 5, 2025 at 10:42 AM Charles Harrison <cshar...@chromium.org> wrote:

Hi folks, context is the IPC review in this CL where Joe helpfully pointed out that sending the full URL to the renderer at WebContentsObserver::ReadyToCommitNavigation time is not safe. We weren't sure however if sending just the origin over is OK or not.

Can anyone please help answer or point me to some documentation?

Thanks,

Charlie

--
--
-----
secu...@chromium.org is for discussing vulnerabilities and fixes in Chromium code.
Please protect Chromium users: DO NOT FORWARD this email or disclose its contents to third parties.
 
http://groups.google.com/a/chromium.org/group/security

To unsubscribe from this group and stop receiving emails from it, send an email to security+u...@chromium.org.

--
--
-----
secu...@chromium.org is for discussing vulnerabilities and fixes in Chromium code.
Please protect Chromium users: DO NOT FORWARD this email or disclose its contents to third parties.
 
http://groups.google.com/a/chromium.org/group/security

[Daniel Cheng]

ReadyToCommitNavigation() is a hook for anything that needs to stash state in the renderer for the navigation that's about to commit. We're not going to pick a different RenderFrameHost to commit the navigation at that point.

Daniel

On Fri, 5 Sept 2025 at 11:27, Charles Harrison <cshar...@chromium.org> wrote:

Alex: I don't think that section addresses my question directly (though it is helpful and the code in question could probably be improved). At the end of the day we still want to send the origin / URL to the renderer before it is committed.

Daniel: The concern was that there are cases where we would do some checks after calling WCO::ReadyToCommit (which is called from here). I am not enough of an expert at the navigation stack to know if this is possible.

[Charles Harrison]

Alex: I don't think that section addresses my question directly (though it is helpful and the code in question could probably be improved). At the end of the day we still want to send the origin / URL to the renderer before it is committed.

Daniel: The concern was that there are cases where we would do some checks after calling WCO::ReadyToCommit (which is called from here). I am not enough of an expert at the navigation stack to know if this is possible.

On Fri, Sep 5, 2025 at 1:18 PM Daniel Cheng <dch...@chromium.org> wrote:

[Joe Mason]

On Fri, Sep 5, 2025 at 2:18 PM Daniel Cheng <dch...@chromium.org> wrote:

I think it's fine to send the full URL. If the renderer were compromised, well, we're about to commit the navigation in it and allow it to read the entire contents of the URL anyway.

I don't think it does send the entire URL contents in all cases. There's code right after ReadyToCommitNavigation that does additional policy checks. I didn't trace through it all, but it wouldn't surprise me if some of them end up cancelling or altering the navigation. And then, right before CommitNavigation, there's this:

  // Before sending the commit parameters to the renderer process, sanitize
  // the redirect URLs to avoid leaking potentially sensitive data into
  // processes which are cross-site. There is no dependency on the
  // cross-site-ness, therefore just sanitize unilaterally.
  SanitizeRedirectsForCommit(commit_params);

... oh, I just realized that sanitizes OTHER url's that are in the commit params (the redirect chain), not the final url.

Of course, if we don't need the full URL, then sending the origin is better.

Agreed. 

[Charles Harrison]

Yeah we definitely try to sanitize redirects (though I think we gave up with subresource redirects IIRC), but I am actually not sure if we do other policy checks at this point in the navigation flow. I just skimmed the code between GetDelegate()->ReadyToCommitNavigation and RenderFrameHost::CommitNavigation and didn't see anything too crazy, mostly just configuring various features.

[Nasko Oskov]

And an extra small data point - we are very close to shipping RenderDocument everywhere (it is 100% on Android now) and what that means is that you get a new RFH for each new document. In such a world, sending the URL before the commit is just sending duplicate data down to the renderer process. I think for me this is more worrisome - having multiple pieces of code deal with supposedly the same information, but allowing for discrepancies to occur and introduce subtle bugs over time. I don't think it is a security issue to send the URL over, since it will be sent in the CommitNavigation IPC anyway, so we aren't disclosing any information to the process that we don't already plan to.

On Fri, 5 Sept 2025 at 15:56, Nasko Oskov <na...@google.com> wrote:

And an extra small data point - we are very close to shipping RenderDocument everywhere (it is 100% on Android now) and what that means is that you get a new RFH for each new document. In such a world, sending the URL before the commit is just sending duplicate data down to the renderer process. I think for me this is more worrisome - having multiple pieces of code deal with supposedly the same information, but allowing for discrepancies to occur and introduce subtle bugs over time. I don't think it is a security issue to send the URL over, since it will be sent in the CommitNavigation IPC anyway, so we aren't disclosing any information to the process that we don't already plan to.