diff --git a/app/controllers/csp_reports_controller.rb b/app/controllers/csp_reports_controller.rb index 30824e3536d..d18fd7d1c2c 100644 --- a/app/controllers/csp_reports_controller.rb +++ b/app/controllers/csp_reports_controller.rb @@ -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? diff --git a/spec/requests/csp_reports_controller_spec.rb b/spec/requests/csp_reports_controller_spec.rb index 87ecbcd6daa..911c30f71aa 100644 --- a/spec/requests/csp_reports_controller_spec.rb +++ b/spec/requests/csp_reports_controller_spec.rb @@ -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