Lukasz Anforowicz | 5d5f887 | 2019-06-14 22:15:04 | [diff] [blame] | 1 | # Use origin (rather than URL) for security decisions. |
| 2 | |
| 3 | URLs are often not sufficient for security decisions, since the origin |
| 4 | may not be present in the URL (e.g., `about:blank`), |
| 5 | may be tricky to parse (e.g., `blob:` or `filesystem:` URLs), |
| 6 | or may be |
| 7 | [opaque](https://html.spec.whatwg.org/multipage/origin.html#concept-origin-opaque) |
| 8 | despite a normal-looking URL (e.g., the security context may be |
| 9 | [sandboxed](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/iframe#attr-sandbox)). |
| 10 | Use origins whenever possible. |
| 11 | |
| 12 | |
| 13 | ## Use `GetLastCommittedOrigin()` or `GetSecurityContext()`. |
| 14 | |
Lukasz Anforowicz | bb73fea | 2022-05-23 22:45:04 | [diff] [blame] | 15 | ### Good |
| 16 | |
Lukasz Anforowicz | 5d5f887 | 2019-06-14 22:15:04 | [diff] [blame] | 17 | ```c++ |
| 18 | // Example of browser-side code. |
| 19 | content::RenderFrameHost* frame = ...; |
| 20 | if (safelist.Matches(frame->GetLastCommittedOrigin()) { |
| 21 | // ... |
| 22 | } |
| 23 | |
| 24 | // Example of renderer-side code. Note that browser-side checks are still |
| 25 | // needed to ensure that a compromised renderer cannot bypass renderer-side-only |
| 26 | // checks. |
| 27 | blink::Frame* frame = ...; |
| 28 | if (safelist.Matches(frame->GetSecurityContext()->GetSecurityOrigin()) { |
| 29 | // ... |
| 30 | } |
| 31 | ``` |
| 32 | |
Lukasz Anforowicz | bb73fea | 2022-05-23 22:45:04 | [diff] [blame] | 33 | ### Bad |
| 34 | |
Lukasz Anforowicz | 5d5f887 | 2019-06-14 22:15:04 | [diff] [blame] | 35 | ```c++ |
| 36 | // Example of browser-side code. |
| 37 | content::RenderFrameHost* frame = ...; |
| 38 | if (safelist.Matches(frame->GetLastCommittedURL()) { |
| 39 | // BUG: doesn't work for about:blank or sandboxed frames. |
| 40 | } |
| 41 | |
| 42 | // Example of renderer-side code. Note that browser-side checks are still |
| 43 | // needed to ensure that a compromised renderer cannot bypass renderer-side-only |
| 44 | // checks. |
| 45 | blink::LocalFrame* frame = ...; |
| 46 | if (safelist.Matches(frame->GetDocument()->Url()) { |
| 47 | // BUG: doesn't work for about:blank or sandboxed frames. |
| 48 | // BUG: doesn't work for RemoteFrame(s) which don't have a local Document |
| 49 | // object and don't know the URL (only the origin) of the frame. |
| 50 | } |
| 51 | ``` |
| 52 | |
| 53 | |
| 54 | ## Don't use the `GURL` type to store origins. |
| 55 | |
| 56 | `GURL origin` is an anti-pattern - representing origins as a URL-focused data |
| 57 | type means that 1) some information is lost (e.g., origin's nonce and precursor |
| 58 | information) and 2) some unnecessary information may be present (e.g., URL path |
| 59 | and/or query parts are never part of an origin). |
| 60 | |
| 61 | Use the following datatypes to represent origins: |
| 62 | |
| 63 | - C++: `url::Origin` or `blink::SecurityOrigin` |
| 64 | (instead of `GURL` or `blink::KURL`). |
| 65 | - Mojo: `url.mojom.Origin` |
| 66 | (instead of `url.mojom.Url`). |
| 67 | Remember to |
| 68 | [validate data](https://chromium.googlesource.com/chromium/src/+/HEAD/docs/security/mojo.md#Validate-privilege_presuming-data-received-over-IPC) |
| 69 | received over IPC from untrustworthy processes |
| 70 | or even better |
| 71 | [avoid sending origins](https://chromium.googlesource.com/chromium/src/+/HEAD/docs/security/mojo.md#Do-not-send-unnecessary-or-privilege_presuming-data) |
| 72 | in the first place. |
Lukasz Anforowicz | 55da3fd | 2019-10-01 16:27:25 | [diff] [blame] | 73 | - Java: `org.chromium.url.Origin` |
Andrew Grieve | 5cec639 | 2023-09-06 14:46:01 | [diff] [blame] | 74 | (see also `url::Origin::FromJavaObject` and `url::Origin::ToJavaObject`). |
Lukasz Anforowicz | 5d5f887 | 2019-06-14 22:15:04 | [diff] [blame] | 75 | |
| 76 | |
| 77 | ## Avoid converting URLs to origins. |
| 78 | |
Lukasz Anforowicz | bb73fea | 2022-05-23 22:45:04 | [diff] [blame] | 79 | ### Good |
| 80 | |
Lukasz Anforowicz | 5d5f887 | 2019-06-14 22:15:04 | [diff] [blame] | 81 | ```c++ |
| 82 | url::Origin origin = GetLastCommittedOrigin(); |
| 83 | ``` |
| 84 | |
Lukasz Anforowicz | bb73fea | 2022-05-23 22:45:04 | [diff] [blame] | 85 | ### Bad |
| 86 | |
Lukasz Anforowicz | 5d5f887 | 2019-06-14 22:15:04 | [diff] [blame] | 87 | ```c++ |
| 88 | GURL url = ...; |
Mike West | 800532c | 2021-10-14 09:26:52 | [diff] [blame] | 89 | GURL origin = url.DeprecatedGetOriginAsURL(); |
Lukasz Anforowicz | 671d2851 | 2022-05-24 22:20:03 | [diff] [blame] | 90 | // BUG: `origin` will be incorrect if `url` is an "about:blank" URL |
| 91 | // BUG: `origin` will be incorrect if `url` came from a sandboxed frame. |
| 92 | // BUG: `origin` will be incorrect when `url` (rather than |
| 93 | // `base_url_for_data_url`) is used when working with loadDataWithBaseUrl |
Lukasz Anforowicz | d39b887 | 2021-04-26 23:13:57 | [diff] [blame] | 94 | // (see also https://crbug.com/1201514). |
Lukasz Anforowicz | 671d2851 | 2022-05-24 22:20:03 | [diff] [blame] | 95 | // BUG: `origin` will be empty if `url` is a blob: URL like |
Lukasz Anforowicz | bb73fea | 2022-05-23 22:45:04 | [diff] [blame] | 96 | // "blob:http://origin/guid-goes-here". |
Lukasz Anforowicz | 671d2851 | 2022-05-24 22:20:03 | [diff] [blame] | 97 | // NOTE: `GURL origin` is also an anti-pattern; see the "Use correct type to |
Lukasz Anforowicz | 5d5f887 | 2019-06-14 22:15:04 | [diff] [blame] | 98 | // represent origins" section below. |
Lukasz Anforowicz | bb73fea | 2022-05-23 22:45:04 | [diff] [blame] | 99 | |
| 100 | // Blink-specific example: |
| 101 | KURL url = ...; |
| 102 | scoped_refptr<SecurityOrigin> origin = SecurityOrigin::Create(url); |
Lukasz Anforowicz | 671d2851 | 2022-05-24 22:20:03 | [diff] [blame] | 103 | // BUG: `origin` will be incorrect if `url` is an "about:blank" URL |
| 104 | // BUG: `origin` will be incorrect if `url` came from a sandboxed frame. |
Lukasz Anforowicz | 5d5f887 | 2019-06-14 22:15:04 | [diff] [blame] | 105 | ``` |
| 106 | |
Lukasz Anforowicz | bb73fea | 2022-05-23 22:45:04 | [diff] [blame] | 107 | ### Risky |
| 108 | |
| 109 | If you know what you are doing and really need to convert a URL into an origin, |
| 110 | then you can consider using `url::Origin::Create`, `url::SchemeHostPort`, or |
| 111 | `url::Origin::Resolve` (noting the limitations of those APIs - see below for |
| 112 | more details). |
| 113 | |
Lukasz Anforowicz | 5d5f887 | 2019-06-14 22:15:04 | [diff] [blame] | 114 | ```c++ |
Lukasz Anforowicz | bb73fea | 2022-05-23 22:45:04 | [diff] [blame] | 115 | const GURL& url = ...; |
| 116 | |
| 117 | // WARNING: `url::Origin::Create(url)` can give unexpected results if: |
Lukasz Anforowicz | 671d2851 | 2022-05-24 22:20:03 | [diff] [blame] | 118 | // 1) `url` is "about:blank", or "about:srcdoc" |
Lukasz Anforowicz | bb73fea | 2022-05-23 22:45:04 | [diff] [blame] | 119 | // (returning unique, opaque origin rather than the real origin of the frame) |
Lukasz Anforowicz | 671d2851 | 2022-05-24 22:20:03 | [diff] [blame] | 120 | // 2) `url` comes from a sandboxed frame |
Lukasz Anforowicz | bb73fea | 2022-05-23 22:45:04 | [diff] [blame] | 121 | // (potentially returning a non-opaque origin, when an opaque one is needed) |
Lukasz Anforowicz | 671d2851 | 2022-05-24 22:20:03 | [diff] [blame] | 122 | // 3) `base_url_for_data_url` should be used instead of `url` |
Lukasz Anforowicz | bb73fea | 2022-05-23 22:45:04 | [diff] [blame] | 123 | // |
| 124 | // WARNING: `url::Origin::Create(url)` has some additional subtleties: |
Lukasz Anforowicz | 671d2851 | 2022-05-24 22:20:03 | [diff] [blame] | 125 | // 4) if `url` is a blob: or filesystem: URL like "blob:http://origin/blob-guid" |
Lukasz Anforowicz | bb73fea | 2022-05-23 22:45:04 | [diff] [blame] | 126 | // then the inner origin will be returned (unlike with `url::SchemeHostPort`) |
| 127 | // 5) data: URLs will be correctly be translated into opaque origins, but the |
| 128 | // precursor origin will be lost (unlike with `url::Resolve`). |
Lukasz Anforowicz | 5d5f887 | 2019-06-14 22:15:04 | [diff] [blame] | 129 | url::Origin origin = url::Origin::Create(url); |
Lukasz Anforowicz | bb73fea | 2022-05-23 22:45:04 | [diff] [blame] | 130 | |
| 131 | // WARNING: `url::SchemeHostPort(url)` will *mechanically* extract the scheme, |
| 132 | // host, and port of the URL, discarding other parts of the URL. This may have |
| 133 | // unexpected results when: |
Lukasz Anforowicz | 671d2851 | 2022-05-24 22:20:03 | [diff] [blame] | 134 | // 1) `url` is "about:blank", or "about:srcdoc" |
| 135 | // 2) `url` comes from a sandboxed frame, i.e. when an opaque origin is expected |
| 136 | // 3) `url` is a data: URL, i.e. when an opaque origin is expected |
| 137 | // 4) `url` is a blob: or filesystem: URL like "blob:http://origin/blob-guid" |
Lukasz Anforowicz | bb73fea | 2022-05-23 22:45:04 | [diff] [blame] | 138 | // (the inner origin will *not* be returned - unlike `url::Origin::Create`) |
| 139 | url::SchemeHostPort scheme_host_port = url::SchemeHostPort(url); |
| 140 | |
Lukasz Anforowicz | 671d2851 | 2022-05-24 22:20:03 | [diff] [blame] | 141 | // `url::Origin::Resolve` should work okay when: |
| 142 | // 1) `url` is "about:blank", or "about:srcdoc" |
| 143 | // 2) `url` comes from a sandboxed frame (i.e. when `base_origin` is opaque) |
| 144 | // 3) `url` is a data: URL (i.e. propagating precursor of `base_origin`) |
| 145 | // 4) `url` is a blob: or filesystem: URL like "blob:http://origin/blob-guid" |
Lukasz Anforowicz | bb73fea | 2022-05-23 22:45:04 | [diff] [blame] | 146 | // |
Lukasz Anforowicz | 671d2851 | 2022-05-24 22:20:03 | [diff] [blame] | 147 | // WARNING: It is simpler and more robust to just use `GetLastCommittedOrigin` |
| 148 | // (instead of combining `GetLastCommittedOrigin` and `GetLastCommittedURL` |
| 149 | // using `url::Origin::Resolve`). |
| 150 | // WARNING: `url::Origin::Resolve` is unaware of `base_url_for_data_url`. |
Lukasz Anforowicz | bb73fea | 2022-05-23 22:45:04 | [diff] [blame] | 151 | const url::Origin& base_origin = ... |
| 152 | url::Origin origin = url::Origin::Resolve(url, base_origin); |
Lukasz Anforowicz | 5d5f887 | 2019-06-14 22:15:04 | [diff] [blame] | 153 | ``` |