Page MenuHomePhabricator

blog-by-user-category message can potentially inject arbitrary strings and parameters when passed to preg_match_all
Closed, ResolvedPublic

Description

In BlogPage.class.php, the getAuthors() method, contains this:

		$categoryName = preg_quote( $categoryName, '/' );
		preg_match_all(
			"/\[\[(?:(?:c|C)ategory|{$categoryName}):\s?" .
				// This is an absolutely unholy, horribly hacky and otherwise
				// less-than-ideal solution that likely works for English only
				// Someone needs to come up with a better solution one of these
				// days than this regex soup...
				str_replace( ' $1', '', wfMessage( 'blog-by-user-category' )->inContentLanguage()->escaped() ) .
			" (.*)\]\]/",
			$articleText,
			$matches
		);

If one customizes the blog-by-user-category message removing with something like "Blogs by user/$1", the str_replace doesn't match and /$1 is inserted directly at the end of preg_match_all, resulting in a PHP Warning: preg_match_all(): Unknown modifier '$'

If one replaces the message with something that ends in /e, it can potentially execute code. (I haven't tested this thoroughly to see if it's possible, but chances are that it can be possible)

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 10 2018, 6:17 PM
Restricted Application added a project: Social-Tools. · View Herald TranscriptAug 10 2018, 6:18 PM

Maybe the message should be passed through preg_quote, and then replace $1 (without spaces) with (.*). Sadly I have no time now to test this :( Maybe in a couple of days...

Note that /e was removed in PHP 7.0 (https://secure.php.net/manual/en/reference.pcre.pattern.modifiers.php), so that part only affects MW < 1.30.

I think but am not sure (have not tested) that this wouldnt work on older versions either due to having other unrecognized "modifiers" (except in some versions if you can inject a null).

In any case, we should fix this for hardening purposes.

I've tested setting blog-by-user-category to end in /e#, and it displays a PHP Warning: preg_match_all(): Unknown modifier '#'. That means the PCRE comment character isn't taken as a comment when present in the modifiers section of the regexp, making it not exploitable (unless a more ancient PHP version allows for that).

Anyway, I'm going to attach the patch here just in case.

Updated patch to account for no match found (looks like the previous one was only visible for me?)

Ciencia_Al_Poder closed this task as Resolved.Aug 26 2018, 6:41 PM

And merged

Legoktm changed the visibility from "Custom Policy" to "Public (No Login Required)".Aug 27 2018, 5:23 AM