Closed Bug 1722252 Opened 3 years ago Closed 3 years ago

"Content Security Policy: Ignoring ‘x-frame-options’ because of ‘frame-ancestors’ directive." warning message even when no "x-frame-options" header present

Categories

(Core :: DOM: Security, defect, P2)

Firefox 90
defect

Tracking

()

RESOLVED FIXED
94 Branch
Tracking Status
firefox94 --- fixed

People

(Reporter: marius.e.nicolae, Assigned: n.goeggi)

Details

(Whiteboard: [domsecurity-active])

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:90.0) Gecko/20100101 Firefox/90.0

Steps to reproduce:

Loaded a specially crafted simple test webpage https://www2.liny.io/ which just displays a message and hosts an iframe to https://www1.liny.io/. The iframe has the 'content-security-policy' header set to "frame-ancestors 'self' https://www2.liny.io;" and no 'x-frame-options' header.

Actual results:

The console displays the warning: "Content Security Policy: Ignoring ‘x-frame-options’ because of ‘frame-ancestors’ directive.", falsely incurring that both headers were detected and the ‘x-frame-options’ being ignored due to‘frame-ancestors’ directive presence.

Expected results:

Detect that 'content-security-policy' header is present but not the 'x-frame-options' and not display the warning at all. Another option would be to display it as info or debug message but clearlly stating the ‘x-frame-options’ header will be ignored only if present. The current message suggest that both headers were detected which might inccur pointless investigations of hunting down the involved headers.

The Bugbug bot thinks this bug should belong to the 'Core::DOM: Security' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Component: Untriaged → DOM: Security
Product: Firefox → Core

I think the behavior is dictated by the ShouldIgnoreFrameOptions function from the FramingChecker.cpp:134 file, where can clearly be seen that only the frame-ancestors header presence is tested.

Niklas, I think we should move the ShouldIgnoreFrameOptions check further down here:
https://searchfox.org/mozilla-central/rev/072710086ddfe25aa2962c8399fefb2304e8193b/dom/security/FramingChecker.cpp#190

Do you mind doing that?

@Mne - thanks for reporting!

Flags: needinfo?(ngogge)
Assignee: nobody → ngogge
Flags: needinfo?(ngogge)
Severity: -- → S4
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Priority: -- → P2
Whiteboard: [domsecurity-active]

Since https://www2.liny.io/ returns a 403 for me i build as separate example site: https://fluoridated-unruly-linseed.glitch.me/
With the attached patch applied the error message is no longer displayed.

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:ngogge, could you have a look please?
For more information, please visit auto_nag documentation.

Flags: needinfo?(ngogge)
Flags: needinfo?(ckerschb)

(In reply to Release mgmt bot [:sylvestre / :calixte / :marco for bugbug] from comment #6)

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.

Thanks Release mgmt bot for spotting that ...

Flags: needinfo?(ngogge)
Flags: needinfo?(ckerschb)
Pushed by mozilla@christophkerschbaumer.com: https://hg.mozilla.org/integration/autoland/rev/f2a411bda53d Check if frame options should be ignored after checking if frame options are present. r=ckerschb
Pushed by mozilla@christophkerschbaumer.com: https://hg.mozilla.org/integration/autoland/rev/40094c67ec8c Check if frame options should be ignored after checking if frame options are present. r=ckerschb

Hi Cristina, these failures seem unrelated to my patch, could you double check if it was really the cause?

Flags: needinfo?(ngogge) → needinfo?(ccozmuta)
Pushed by mozilla@christophkerschbaumer.com: https://hg.mozilla.org/integration/autoland/rev/50260304b0f0 Check if frame options should be ignored after checking if frame options are present. r=ckerschb
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 94 Branch
Flags: needinfo?(ccozmuta)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: