FIX: stop logging blank and invalid CSP reports (#17144)

Certain rogue bots such as Yandex may send across invalid CSP reports
when CSP report collection is enabled.

This ensures that invalid reports will not cause log floods and simply
returns a 422 error.

Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
This commit is contained in:
Sam 2022-06-20 16:57:46 +10:00 committed by GitHub
parent 5176c689e9
commit 9361d9a587
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 44 additions and 17 deletions

View File

@ -5,28 +5,43 @@ class CspReportsController < ApplicationController
def create
raise Discourse::NotFound unless report_collection_enabled?
Logster.add_to_env(request.env, 'CSP Report', report)
Rails.logger.warn("CSP Violation: '#{report['blocked-uri']}' \n\n#{report['script-sample']}")
report = parse_report
head :ok
if report.blank?
render_json_error("empty CSP report", status: 422)
else
Logster.add_to_env(request.env, 'CSP Report', report)
Rails.logger.warn("CSP Violation: '#{report['blocked-uri']}' \n\n#{report['script-sample']}")
head :ok
end
rescue JSON::ParserError
render_json_error("invalid CSP report", status: 422)
end
private
def report
@report ||= JSON.parse(request.body.read)['csp-report'].slice(
'blocked-uri',
'disposition',
'document-uri',
'effective-directive',
'original-policy',
'referrer',
'script-sample',
'status-code',
'violated-directive',
'line-number',
'source-file'
)
def parse_report
obj = JSON.parse(request.body.read)
if Hash === obj
obj = obj['csp-report']
if Hash === obj
obj.slice(
'blocked-uri',
'disposition',
'document-uri',
'effective-directive',
'original-policy',
'referrer',
'script-sample',
'status-code',
'violated-directive',
'line-number',
'source-file'
)
end
end
end
def report_collection_enabled?

View File

@ -32,6 +32,18 @@ describe CspReportsController do
}.to_json, headers: { "Content-Type": "application/csp-report" }
end
it 'returns an error for invalid reports' do
SiteSetting.content_security_policy_collect_reports = true
post '/csp_reports', params: "[ not-json", headers: { "Content-Type": "application/csp-report" }
expect(response.status).to eq(422)
post '/csp_reports', params: ["yes json"].to_json, headers: { "Content-Type": "application/csp-report" }
expect(response.status).to eq(422)
end
it 'is enabled by SiteSetting' do
SiteSetting.content_security_policy = false
SiteSetting.content_security_policy_report_only = false