Page MenuHomePhabricator

Mediawiki replies with 500 on wrongly formatted CSP report
Closed, ResolvedPublic

Description

Noticed this when looking at the list of 500s, wmfwiki receives some malformed csp reports to which mediawiki replies with 500, IMO it should be a 400 since it is client-provided data we are trying to parse.

"message": "Error reading CSP report: wrongformat",
 "url": "/w/api.php?action=cspreport&format=none&reportonly=1&source=wmfwiki&",
 "method": "ApiCSPReport::verifyPostBodyOk",
 "channel": "csp-report-only",
 "reqId": "WSWDmQpAEDEAAULovFQAAADG",

Event Timeline

BBlack added a subscriber: BBlack.May 24 2017, 5:12 PM

Agreed this should be 4xx rather than 5xx

Restricted Application added a project: Operations. · View Herald TranscriptMay 24 2017, 5:13 PM

There's a code comment that says:

		// 500 so it shows up in browser's developer console.
		$this->dieWithError(
			[ 'apierror-csp-report', wfEscapeWikiText( $code ) ], 'cspreport-' . $code, [], 500
		);

doh :( I'm for changing that to 400, we log those anyways on our side, thoughts @Anomie @Bawolff ?

Yes, 400 would seem to be more semantically appropriate than 500. I have no idea whether a 400 response also shows up in a browser's developer console.

@Anomie yeah I don't know either what would happen in that case, I think we can turn it into a 400 for now though. Will send a patch for mw-core.

Change 355757 had a related patch set uploaded (by Filippo Giunchedi; owner: Filippo Giunchedi):
[mediawiki/core@master] Return 400 on invalid CSP reports

https://gerrit.wikimedia.org/r/355757

fgiunchedi moved this task from Backlog to Radar on the User-fgiunchedi board.
ema moved this task from Triage to Watching on the Traffic board.May 29 2017, 9:07 AM

Change 355757 merged by jenkins-bot:
[mediawiki/core@master] Return 400 on invalid CSP reports

https://gerrit.wikimedia.org/r/355757

matmarex closed this task as Resolved.May 29 2017, 5:42 PM
matmarex assigned this task to fgiunchedi.
matmarex removed a project: Patch-For-Review.
Jdforrester-WMF added a subscriber: Jdforrester-WMF.

Mass-moving all items tagged for MediaWiki 1.30.0-wmf.3, as that was never released; instead, we're using -wmf.4.

sbassett moved this task from Backlog to Done on the Security-Team board.Jun 11 2019, 6:32 PM