“about:blank#blocked”

[Avi Drissman]

Re https://crbug.com/1255713 (security bug).

One of the problems in this bug comes from a line:

url::Origin::Create(web_contents->GetLastCommittedURL())

In general, that's an antipattern, but what's happening specifically here is that GetLastCommittedURL() is returning "about:blank#blocked", which Origin is turning into a default-constructed Origin.

(Specifically, "about:blank#blocked" is passed to the SchemeHostPort constructor, which calls IsValidInput(), which returns false as "about" isn't allowed.)

This allows laundering of origins for anyone using that antipattern.

I'm not sure what to ask here. Can we make the url::Origin::Create call smarter? Can we do something on the "about:blank#blocked" side? Can we just remove url::Origin::Create?

[Avi Drissman]

Also, the number of results from https://source.chromium.org/search?q=Origin::Create.*LastCommittedURL&ss=chromium is disappointing.

[Daniel Cheng]

On Fri, 8 Oct 2021 at 12:56, 'Avi Drissman' via navigation-dev <navigat...@chromium.org> wrote:

Re https://crbug.com/1255713 (security bug).

One of the problems in this bug comes from a line:

url::Origin::Create(web_contents->GetLastCommittedURL())

In general, that's an antipattern, but what's happening specifically here is that GetLastCommittedURL() is returning "about:blank#blocked", which Origin is turning into a default-constructed Origin.

(Specifically, "about:blank#blocked" is passed to the SchemeHostPort constructor, which calls IsValidInput(), which returns false as "about" isn't allowed.)

This allows laundering of origins for anyone using that antipattern.

What does "laundering of origins" mean? The default url::Origin constructor creates a unique opaque origin, which seems like a reasonable default. What other origin should it infer?

Daniel

 

I'm not sure what to ask here. Can we make the url::Origin::Create call smarter? Can we do something on the "about:blank#blocked" side? Can we just remove url::Origin::Create?

--
You received this message because you are subscribed to the Google Groups "navigation-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to navigation-de...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/navigation-dev/CACWgwAa2G7nRnGgShNH6MpmD5%3DuhTC4Xe%2ByxT2HCTFssWEzmdQ%40mail.gmail.com.

[Avi Drissman]

That specific bug is marked as a security bug, so look at it for details, but a page is deliberately tripping the FilterURL check, turning its URL to "about:blank#blocked". Then bad code in Chromium builds an Origin from the URL, and then uses the origin.

That's what I mean by a laundered origin. The page's origin should be related to the original URL, either matching the scheme/host/port, or being opaque but being derived from that scheme/host/port. But by deliberately tripping the FilterURL check, the page ends up with an "origin" that is completely unrelated to its real URL.

Can we somehow prohibit that bad code?

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/navigation-dev/CAF3XrKr2Pf-3qRbgPdTUvfYwqK9ZCCkOwovLaEWV6vGo-jGwGQ%40mail.gmail.com.

[Łukasz Anforowicz]

So I guess the goal here is to make it impossible to take the thing returned from GetLastCommittedURL and pass it into url::Origin:Create, right?

Silly ideas in no particular order:

• Rename url::Origin::Create into url::Origin::UnsafeCreateIPromiseThisIsNotAboutURLNorBlobURLNor...
• Make GetLastCommittedURL return LastCommittedURL rather than GURL (LastCommittedURL being derived from GURL?) and having no overload of url::Origin::Create that takes LastCommittedURL.
• Get rid of GetLastCommittedURL and GetLastCommittedOrigin and instead return a single object (LastCommittedUrlAndOrigin?) with 1) origin getter, 2) *no* GURL getter (maybe just a way to format the URL + match the URL somehow (IsPath(...), IsUrlMatchingPatter(...), IsUrlOrOriginHostEqualTo(...), etc.)

I am not sure if any of the ideas above are any good (they probably aren't) but maybe together we can come up with something doable...

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/navigation-dev/CACWgwAa%2BQjDbGn8mW6a_xDJQaO51giaBVgWWJsuYwxu8NiHzFA%40mail.gmail.com.

[Avi Drissman]

Urgh, yeah, they're all not great. A part of me wonders if just doing a presubmit on /Origin::Create.*LastCommittedURL/ would work.

Y'all have been working on Origin longer than I have; should all of these https://source.chromium.org/search?q=Origin::Create.*LastCommittedURL&ss=chromium be rewritten to web_contents->GetMainFrame()->GetLastCommittedOrigin()?