Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Fix CSP report-uri directive defaulting to 'none' when empty value provided #93

Merged
merged 1 commit into from
Jan 19, 2017

Conversation

tkjn
Copy link

@tkjn tkjn commented Oct 29, 2016

If setDirective is called on \Zend\Http\Header\ContentSecurityPolicy with an empty array for report-uri the resulting header contains report-uri: 'none'

$csp->setDirective('report-uri', []);

According to CSP2 specification https://www.w3.org/TR/CSP2/ the report-uri directive is not a source-list, it actually accepts 1 or more uri-reference. This means the default of 'none' is not treated as no report-uri.

If report-uri is defaulted to 'none', I have observed CSP errors being reported to an endpoint of host/'none'

Screenshot of Network tab in both Chrome and Firefox with report-uri:'none':
csp-report-uri-none

My proposed fix will ensure that the report-uri directive is unset if an empty array is provided rather than defaulting to 'none'. This will then omit the report-uri from the ContentSecurityPolicy header which achieves what I would expect for an empty report-uri

I have created branches of ZendSkeletonApplication.
One which highlights the error
https://github.com/tkjn/ZendSkeletonApplication/tree/csp-report-uri-none-error
And another which uses my proposed fix
https://github.com/tkjn/ZendSkeletonApplication/tree/csp-report-uri-none-error-fixed

@tkjn tkjn changed the title CSP report-uri directive defaults to 'none' when empty value provided Fix CSP report-uri directive defaulting to 'none' when empty value provided Dec 14, 2016
@ezimuel ezimuel self-assigned this Jan 19, 2017
@ezimuel ezimuel added the bug label Jan 19, 2017
@ezimuel ezimuel merged commit be696d6 into zendframework:master Jan 19, 2017
@ezimuel
Copy link
Contributor

ezimuel commented Jan 19, 2017

@tkjn thanks for the PR!

ezimuel added a commit that referenced this pull request Jan 19, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants