Page MenuHomePhabricator

Fix security issues found in Graphs extension during review of vega 2
Open, NormalPublic

Description

Split from other bug

  • For CORS, in the interest of paranoia, could we set the origin parameter to be * instead of the actual origin. This wasn't available when the wrapper was originally written, but it extra ensures that cross-domain requests won't use cookies.
  • Currently the title parameter is checked to make sure it doesn't have a | (pipe) character in it. We also need to check for \x1f (That is ASCII control character UNIT SEPARATOR) as the api also uses that to separate titles. Alternatively we could validate with mw.Title
  • wikititle:/// is supposed to prevent query paramters from being used, however it could probably be bypassed if they are percent encoded due to T96274 (e.g. https://en.wikipedia.org/wiki/Main_Page%3faction=history%26curid=2120 is interpreted by our servers incorrectly )

Details

Related Gerrit Patches:
mediawiki/extensions/Graph : masterHardening changes. Cors use *, no \x1F in wikiraw

Event Timeline

Bawolff triaged this task as Normal priority.Dec 10 2017, 3:56 PM
Bawolff created this task.
Bawolff created this object with visibility "Custom Policy".
Aklapper renamed this task from Fix security issues found in Graphs extension duing review of vega 2 to Fix security issues found in Graphs extension during review of vega 2.Dec 11 2017, 1:21 PM
Bawolff changed the visibility from "Custom Policy" to "Public (No Login Required)".May 8 2019, 5:37 PM

Change 508872 had a related patch set uploaded (by Brian Wolff; owner: Brian Wolff):
[mediawiki/extensions/Graph@master] Hardening changes. Cors use *, no \x1F in wikiraw

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

wikititle:/// is supposed to prevent query paramters from being used, however it could probably be bypassed if they are percent encoded due to T96274 (e.g. https://en.wikipedia.org/wiki/Main_Page%3faction=history%26curid=2120 is interpreted by our servers incorrectly )

HHVM is going away shortly anyways, and php7 does not have this issue.