Page MenuHomePhabricator

Parameters injection in SyntaxHighlight results in multiple vulnerabilities
Closed, ResolvedPublic

Description

As reported by Yorick Koster:

To: security@wikimedia.org
From: Yorick Koster 
Subject: SyntaxHighlight MediaWiki exension allows injection of arbitrary Pygments options
Organization: Securify B.V.

Hi,

I've found a security vulnerability in the SyntaxHighlight extension. 
The details are attached. A quick fix would be to cast the start 
parameter to int.

// Starting line number
if ( isset( $args['start'] ) ) {
    $options['linenostart'] = (int)$args['start'];
}

Cheers,

Yorick

-- 

Yorick Koster
Co-founder
Securify

Original e-mail attachments:

Event Timeline

Thanks @Reedy. Is there a SAL entry associated with this deployment?

Thanks @Reedy. Is there a SAL entry associated with this deployment?

Not exactly. I got it staged in time so that it went out with @demon's scaps for .13

Reedy claimed this task.

Closing for ease of tracking progress. Patches attached to parent bug, due for next release

Reedy changed the visibility from "Custom Policy" to "Public (No Login Required)".Apr 6 2017, 8:57 PM

Change 346868 had a related patch set uploaded (by Chad; owner: Reedy):
[mediawiki/extensions/SyntaxHighlight_GeSHi@master] SECURITY: Escape start argument before passing to pygments

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

Change 346870 had a related patch set uploaded (by Chad; owner: Reedy):
[mediawiki/extensions/SyntaxHighlight_GeSHi@REL1_28] SECURITY: Escape start argument before passing to pygments

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

Change 346871 had a related patch set uploaded (by Chad; owner: Reedy):
[mediawiki/extensions/SyntaxHighlight_GeSHi@REL1_27] SECURITY: Escape start argument before passing to pygments

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

Change 346868 merged by jenkins-bot:
[mediawiki/extensions/SyntaxHighlight_GeSHi@master] SECURITY: Escape start argument before passing to pygments

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

Change 346870 merged by jenkins-bot:
[mediawiki/extensions/SyntaxHighlight_GeSHi@REL1_28] SECURITY: Escape start argument before passing to pygments

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

Change 346871 merged by jenkins-bot:
[mediawiki/extensions/SyntaxHighlight_GeSHi@REL1_27] SECURITY: Escape start argument before passing to pygments

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

Change 347015 had a related patch set uploaded (by Chad; owner: Reedy):
[mediawiki/extensions/SyntaxHighlight_GeSHi@wmf/1.29.0-wmf.19] SECURITY: Escape start argument before passing to pygments

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

Change 347015 merged by jenkins-bot:
[mediawiki/extensions/SyntaxHighlight_GeSHi@wmf/1.29.0-wmf.19] SECURITY: Escape start argument before passing to pygments

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

This issue is reported as fixed in 1.28.1 / 1.27.2, but I can't seem to find the fix.

https://lists.wikimedia.org/pipermail/mediawiki-announce/2017-April/000207.html

@demon @Reedy how to proceed? The details of this issue are now public, yet the fix is not included in 1.28.1 & 1.27.2

They're not in the tarballs, but they are in git

The question​ is whether the tarballs for both should be reissued

Fair enough, although I kinda got that :). My point is that a lot of people will use the tarball and will not get the fix (if they update at all). The fix is for example also not in Debian's version (https://sources.debian.net/src/mediawiki/1:1.27.2-1/extensions/SyntaxHighlight_GeSHi/SyntaxHighlight_GeSHi.class.php/).

The question​ is whether the tarballs for both should be reissued

Please don't do that. I understand the appeal but that would create a world of pain and incomprehension between people having the first and the second version of the tarball, which would be especially bad because we are talking about a fix resolving a security issue. Speaking with my Arch security team hat on, it would also break reproducible builds if the upstream tarball changes. Please consider issuing 1.28.2 and 1.27.3 instead.

We should really issue another release for this right away. The syntax highlight bug was by far the most severe of all the bugs last release. To avoid confusion we should probably bump the mediawiki version number (eventhough that version number technically only applies to mediawiki core and not syntaxhighlight)