Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When REPORT_PERCENTAGE is set to 100%, the report-uri directive is not always included in the CSP header #231

Closed
robhudson opened this issue Jun 27, 2024 · 2 comments

Comments

@robhudson
Copy link
Member

When the setting REPORT_PERCENTAGE is set to 100%, the report-uri should always be included in the Content Security Policy header. However, it has been observed that this is not true.

The current logic allows this issue to occur because when the randomly generated integer equals 100, the condition to include the report-uri is not met. Specifically, the check 100 < 100 evaluates to False, which results in the report-uri being omitted from the CSP header. This effectively means that even with REPORT_PERCENTAGE set to 100%, there is a scenario where the report-uri is not included, preventing the expected reporting of CSP violations.

        include_report_uri = random.randint(0, 100) < report_percentage
        if not include_report_uri:
            replace["report-uri"] = None
@crbunney
Copy link
Contributor

crbunney commented Jul 5, 2024

I've also encountered this issue on 4.0b1. report-to in contrast behaves as expected and is added to the CSP policy header

@robhudson
Copy link
Member Author

I've also encountered this issue on 4.0b1. report-to in contrast behaves as expected and is added to the CSP policy header

This is because the code only removes the report_uri directive and doesn't also remove report-to. Probably because report-to was added relatively recently and likely got missed here. I'd consider this to be a related bug.

The report-to directive requires a reporting endpoint as the value and is defined in a separate header. Are you defining that header elsewhere in some way? I just opened #235 to consider having django-csp set the reporting endpoints.

robhudson added a commit that referenced this issue Jul 9, 2024
This also updates the RateLimited CSPMiddleware to remove both
`report-uri` and `report-to` directives based on report percentage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants