Bug 185120 - From-Origin load violation in network process should make main resource loads look successful
Summary: From-Origin load violation in network process should make main resource loads...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-04-29 10:18 PDT by Daniel Bates
Modified: 2019-12-16 02:21 PST (History)
19 users (show)

See Also:


Attachments
Patch (28.81 KB, patch)
2018-05-04 13:47 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (60.75 KB, patch)
2019-12-15 07:28 PST, Rob Buis
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2018-04-29 10:18:55 PDT
Follow up on my remark in bug 184560, comment 62:

[[
From my understanding From-Origin can be used to achieve the same effect as X-Frame-Options when delivered in the HTTP response to a document. We need to take the same approach as we do for handling a X-Frame-Options violation and make it appear that the page did load successfully because failing the load the way this code does reveals the existence of the document, which is sensitive. In particular, we need to cancel the load, logging a console message, sandboxing the iframe that load was destined for and dispatching a DOM load event to the destination frame. For subresources we need to keep the current behavior and fail the load.

A side benefit of taking this approach is that we can remove the use of the setTimeout() from the test sandboxed-sub-frame-from-origin-same-blocked.html and instead end the test once we get a DOM load event for the frame.
]]
Comment 1 Radar WebKit Bug Importer 2018-04-29 11:48:41 PDT
<rdar://problem/39825500>
Comment 2 Daniel Bates 2018-04-30 12:20:36 PDT
John Wilander brought this issue up in <https://github.com/whatwg/fetch/issues/687>.
Comment 3 Daniel Bates 2018-04-30 12:23:09 PDT
(In reply to Daniel Bates from comment #2)
> John Wilander brought this issue up in
> <https://github.com/whatwg/fetch/issues/687>.

Specifically, it was brought up in <https://github.com/whatwg/fetch/issues/687#issuecomment-385470743>.
Comment 4 Daniel Bates 2018-05-04 13:47:40 PDT
Created attachment 339589 [details]
Patch
Comment 5 youenn fablet 2018-05-04 14:57:37 PDT
From-Origin is not specific to main resource loads like X-Frame-Options is.
Having a behavior different according the destination does not seem appealing to me.

I would go with either keeping the current behavior or moving all loads to CORB filtered empty responses in case From-Origin is violated. I am not clear of the benefit of the latter approach though.

Can you clarify why revealing the existence of the document is sensitive?
Is it sill an issue if we make sure that the X-Frame-Options check is made before the From-Origin check?
Comment 6 Geoffrey Garen 2018-05-04 15:13:52 PDT
Comment on attachment 339589 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=339589&action=review

> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:443
> +        // Only subresources can fail to load. Main resources must look like they loaded successfully or we leak their existence.

Let's move this comment above, to explain the special case for main resources. I would just say "We treat a canceled load of a main resource as if it succeeded as a way to prevent main resource loads from scanning for URLs".

NOTE: I'm not sure how effective this check is, given that it doesn't apply to sub resources. If I were the attacker, I would just use a subresource loads for my attack.
Comment 7 Geoffrey Garen 2018-05-04 16:05:54 PDT
Comment on attachment 339589 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=339589&action=review

>> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:443
>> +        // Only subresources can fail to load. Main resources must look like they loaded successfully or we leak their existence.
> 
> Let's move this comment above, to explain the special case for main resources. I would just say "We treat a canceled load of a main resource as if it succeeded as a way to prevent main resource loads from scanning for URLs".
> 
> NOTE: I'm not sure how effective this check is, given that it doesn't apply to sub resources. If I were the attacker, I would just use a subresource loads for my attack.

I think we have two issues with this change in behavior, to make From-Origin treat the main resource as special:

(1) It seems like Mozilla may not support this behavior, based on the comments around <https://github.com/whatwg/fetch/issues/687#issuecomment-385896423>.

(2) We don't have a clear explanation of the security benefit of applying a behavior to main resources and not sub resources.

I think it's probably best not to apply this quirk to From-Origin until we can explain its benefit. If we can explain its benefit, we can probably convince other browser vendors too. Or we might learn that we should apply this behavior to all resources.
Comment 8 Rob Buis 2019-12-15 07:28:40 PST
Created attachment 385719 [details]
Patch