New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 798698 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression in Chrome 64: CSP directive form-action 'self' prevents <form target="_blank"> submit

Reported by steffen....@gmail.com, Jan 3 2018

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3239.108 Safari/537.36

Steps to reproduce the problem:
1. Open https://www.computerbase.de/chrome-form-target-blank-csp-bug/
2. Click "Submit"

What is the expected behavior?
Chrome should open a new tab and load the page

What went wrong?
Chrome opens a new tab that is completely empty. The developer tools log the following error:

Refused to send form data to 'https://www.computerbase.de/suche/?q=foo' because it violates the following Content Security Policy directive: "form-action 'self'".

Did this work before? N/A 

Does this work in other browsers? N/A

Chrome version: 64.0.3282.39  Channel: beta
OS Version: 
Flash Version: 

The page is served with the CSP directive "form-action 'self'". This has worked fine up to Chrome 63. It doesn't work anymore in Chrome 64 and 65 and I'm quiet sure this is a bug.
 
The bug disappears if you remove the target="_blank" attribute.
Labels: Needs-Triage-M64

Comment 3 Deleted

Cc: mkwst@chromium.org
Labels: -Type-Bug -Pri-2 ReleaseBlock-Stable M-64 Needs-Bisect Needs-Triage-M64 OS-Linux OS-Mac Pri-1 Type-Bug-Regression
Owner: andypaicu@chromium.org
Status: Assigned (was: Unconfirmed)
Able to reproduce the issue with latest Chrome beta i.e., 64.0.3282.71 on Windows, Mac and Linux. Please find the regression range below :

You are probably looking for a change made after 518244 (known good), but no later than 518245 (first known bad).
CHANGELOG URL:
The script might not always return single CL as suspect as some perf builds might get missing due to failure.
  https://chromium.googlesource.com/chromium/src/+log/daaa9faed34a50b6d218eb0f4c1f4f9b7de59885..209f225b2d51334eaf69ffdf002e25eaa1e0d448
Cc: abdulsyed@chromium.org manoranj...@chromium.org
Labels: -Needs-Bisect -Needs-Triage-M64 M-65 FoundIn-64 FoundIn-65 Target-64
andypaicu@ Gentle Ping! This issue is marked as RB-Stable, could you please let us know is there any latest update available on this issue?

Comment 7 by mkwst@chromium.org, Jan 8 2018

Cc: andypaicu@chromium.org
Components: UI>Browser>Navigation
Owner: arthurso...@chromium.org
If it's `form-action`, and started in M64, it might well be PlzNavigate. Arthur, would you mind taking a look?
PlzNavigate was enabled on M61 on all platforms (except for WebView on M63). So this is not caused by enabling PlzNavigate, but something after that.

I tried:
63.0.3239.108 (Official Build) (64-bit) -> Do not reproduce the bug.
65.0.3316.0 (Developer Build) (64-bit) -> Can reproduce the bug.

I will try to understand the issue. Thanks "steffen.weber" for the repro steps!

Comment 9 Deleted

I understand the issue.

1) A RenderFrameImpl display a page with "form-action 'self'" CSP and
   self="https://www.computerbase.de/chrome-form-target-blank-csp-bug/
2) A form submission happens with target="_blank"
3) A new RenderFrameImpl is created and self is undefined.
4) CSP checks fails.

The issue is that the form-action is checked against the CSP of the navigating frame, not the one that has initiated the navigation in the first place. This is the current behavior and it has always been the case. I submitted a bug a while ago about this here: issue 700964

This doesn't explain why this bug didn't occur on M63. I will do a bisection in order to understand why it was working before.

I think it was working before because the CSP was not propagated to the second
RenderFrameImpl.

It think it was fixed by:
https://chromium.googlesource.com/chromium/src/+/209f225b2d51334eaf69ffdf002e25eaa1e0d448.
This bug probably starts failing after this CL.

I am considering options we have.
1) Maybe we can do browser side checks only on redirects and rely on the
   renderer side checks for the initial url.
2) Find a way to get the RenderFrameHost that has initiated the navigation and
   check CSP on it instead of the new one. That would be the right way, but I
   believe it is complicated.
3) We have now more confidence in BeginNavigationParams::initiator_origin to be
   correct. It could be used to be 'self' in this case.
4) Find a way to call RenderFrameHostImpl::SetSelf([...]) when the
   RenderFrameHostImpl is constructed from a form submission with
   target="_blank".

Do you have any suggestions?

1) is the easy way.
2) is the complicated and correct way.
I don't have any opinions on 3) and 4). I need to take a closer look.
RE: #c10

AFAICT, https://www.w3.org/TR/CSP3/#directive-form-action says that the the form-action directive should apply to "the target of a form submissions" (not to the initiator of the navigation as mentioned in #c10).  Please also note the test cases added in https://codereview.chromium.org/2464123004 (e.g. third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-blocked-when-target-blank.html).
Re: #c11
If that's true, that would be very nice for fixing this bug (but very weird too).

When I read https://www.w3.org/TR/CSP3/#directive-form-action, I still understand what I said in #c10.
Let me formalize what I am taking about:
Let A and B to be 2 frames.
1) A does a form submit and target B (with url X).
2) Then B starts a navigation and checks the form-action CSP.
=> It must check if the url X match A's CSP (and not B's CSP)
Thanks for the clarification in #c13 - that makes perfect sense now.

See also  https://crbug.com/630332#c20  and https://github.com/w3c/webappsec-csp/issues/8 where I try to argue that form-action should focus on blocking form->immediate-target-url (and potentially ignore form->final-redirect-destination-of-target-url).  AFAIR, the fix in  https://crbug.com/630332#c28  would ignore the final-redirect form->final-redirect-destination-of-target-url in case of OOPIFs (i.e. AFAIR the fix adds checking of the URL in A's process, using the terminology from #c13;  also - AFAIR I was hoping to eventually remove the checks in B's process after the w3c discussion linked above is resolved, but this never happened :-( - see issue 663512).
Cc: est...@chromium.org
Status: Started (was: Assigned)
I started a CL. I have a test and a fix that does the 1) options.
I still need to update:
http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-*
I introduced these tests to document what is the current behavior (not necessary correct BTW)


Friendly reminder: M64 Stable is only 11 days away, and last M64 beta is next week. 
FYI: The CL is here:
https://chromium-review.googlesource.com/c/chromium/src/+/857503
It needs LGTMs though.

This really need to be merged into the next (and last) M64 beta.
Project Member

Comment 19 by bugdroid1@chromium.org, Jan 15 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/078468ed4f8ad666f7436086cd8c069c9ae414ec

commit 078468ed4f8ad666f7436086cd8c069c9ae414ec
Author: arthursonzogni <arthursonzogni@chromium.org>
Date: Mon Jan 15 12:27:51 2018

CSP: Fix blocked form submission with target="_blank"

This fixes  bug 798698 . It happens when the "form-action 'self'" CSP is
used and a form submission with the attribute "target='_blank'" happens.
Since the form-action CSP is checked against the CSP of the new windows
instead of the one of the old window, it was blocked.

This regression probably started after this CL (which is good):
https://chromium-review.googlesource.com/c/chromium/src/+/765969.

Solution was to disable browser side checks and only rely on renderer
side checks for the initial load.

Bug:  798698 
Change-Id: Iade17b80f493af265ddb86fe95305d96c7ce0975
Reviewed-on: https://chromium-review.googlesource.com/857503
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529255}
[modify] https://crrev.com/078468ed4f8ad666f7436086cd8c069c9ae414ec/content/browser/frame_host/form_submission_throttle.cc
[modify] https://crrev.com/078468ed4f8ad666f7436086cd8c069c9ae414ec/content/browser/frame_host/form_submission_throttle.h
[modify] https://crrev.com/078468ed4f8ad666f7436086cd8c069c9ae414ec/content/browser/frame_host/form_submission_throttle_browsertest.cc
[delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/content/test/data/form_submission_throttle/form_action_with_path.html
[add] https://crrev.com/078468ed4f8ad666f7436086cd8c069c9ae414ec/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/form-action/form-action-self-allowed-target-blank.html
[add] https://crrev.com/078468ed4f8ad666f7436086cd8c069c9ae414ec/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/support/postmessage-pass-to-opener.html
[delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-child-expected.txt
[delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-child.html
[delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-parent-expected.txt
[delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-parent.html
[delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-main-page-expected.txt
[delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-main-page.html
[delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-window-reload-from-child-expected.txt
[delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-window-reload-from-child.html
[delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-window-reload-from-parent-expected.txt
[delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-window-reload-from-parent.html
[delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/form-action-none-and-reload.html
[delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/form-action-none.html
[delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/form-action-resubmission-main-page-callee.html
[delete] https://crrev.com/d659b8182fc16950711020a0e1b797575500949e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/form-action-resubmission-main-page-caller.html

Labels: Merge-Request-64
I request a merge for M64.

I prepared the cherry-pick here:
https://chromium-review.googlesource.com/c/chromium/src/+/866750
Project Member

Comment 21 by sheriffbot@chromium.org, Jan 15 2018

Labels: -Merge-Request-64 Hotlist-Merge-Review Merge-Review-64
This bug requires manual review: We are only 7 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: cma...@chromium.org
Can we merge this CL?

I know that the next (and last) M64 beta will be built today. This CL really needs to be included into it.

I didn't get any answer yesterday, probably because of public holiday in the U.S.
I will leave the office now (EMEA). Please, feel free to merge it once it it approved:
https://chromium-review.googlesource.com/c/chromium/src/+/866750
Thanks Arthur for the fix! Can you please comment on why this is absolutely urgent? What are the implications if we wait until M65? How safe is this merge and is this well tested, and are we absolutely sure this will not introduce any new regressions?
If we don't merge this CL. Website owner that are using the "form-action" CSP will see form submission blocked when it targets a new window.
Big website are using CSP, so I think it will break several of them.
I think it is severe.

I am confident that it will not introduce new regression. The change is only 2 line of code. If there is a regression, it will be a navigation not being blocked. The website owner don't expect this navigation to happen in the first place (CSP is a defensive feature.)
This CL disable browser side checks for the initial load. Those checks still happen on the renderer side. The modified class is here almost only for checking CSP on redirects. This part is not removed.
Labels: -Merge-Review-64 Merge-Approved-64
Approving merge for M64 based on #23 and #24. Branch:3282
Project Member

Comment 26 by bugdroid1@chromium.org, Jan 16 2018

Labels: -merge-approved-64 merge-merged-3282
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6426a279940d16afb442b2c261362f31af6c2712

commit 6426a279940d16afb442b2c261362f31af6c2712
Author: arthursonzogni <arthursonzogni@chromium.org>
Date: Tue Jan 16 18:58:53 2018

CSP: Fix blocked form submission with target="_blank"

This fixes  bug 798698 . It happens when the "form-action 'self'" CSP is
used and a form submission with the attribute "target='_blank'" happens.
Since the form-action CSP is checked against the CSP of the new windows
instead of the one of the old window, it was blocked.

This regression probably started after this CL (which is good):
https://chromium-review.googlesource.com/c/chromium/src/+/765969.

Solution was to disable browser side checks and only rely on renderer
side checks for the initial load.

Bug:  798698 
Change-Id: Iade17b80f493af265ddb86fe95305d96c7ce0975
Reviewed-on: https://chromium-review.googlesource.com/857503
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#529255}(cherry picked from commit 078468ed4f8ad666f7436086cd8c069c9ae414ec)
Reviewed-on: https://chromium-review.googlesource.com/866750
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/branch-heads/3282@{#508}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[modify] https://crrev.com/6426a279940d16afb442b2c261362f31af6c2712/content/browser/frame_host/form_submission_throttle.cc
[modify] https://crrev.com/6426a279940d16afb442b2c261362f31af6c2712/content/browser/frame_host/form_submission_throttle.h
[modify] https://crrev.com/6426a279940d16afb442b2c261362f31af6c2712/content/browser/frame_host/form_submission_throttle_browsertest.cc
[delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/content/test/data/form_submission_throttle/form_action_with_path.html
[add] https://crrev.com/6426a279940d16afb442b2c261362f31af6c2712/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/form-action/form-action-self-allowed-target-blank.html
[add] https://crrev.com/6426a279940d16afb442b2c261362f31af6c2712/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/support/postmessage-pass-to-opener.html
[delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-child-expected.txt
[delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-child.html
[delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-parent-expected.txt
[delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-iframe-reload-from-parent.html
[delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-main-page-expected.txt
[delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-main-page.html
[delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-window-reload-from-child-expected.txt
[delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-window-reload-from-child.html
[delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-window-reload-from-parent-expected.txt
[delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/form-action-resubmission-window-reload-from-parent.html
[delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/form-action-none-and-reload.html
[delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/form-action-none.html
[delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/form-action-resubmission-main-page-callee.html
[delete] https://crrev.com/b7040e0bd25998bf16cae683de3b575dc18d7c26/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/form-action-resubmission-main-page-caller.html

Status: Fixed (was: Started)
Thank you! :)

Sign in to add a comment