WordPress.org

Support

Support » Plugins and Hacks » mfunc issue

mfunc issue

  • Dear wp-super-cache/w3-total-cache developers,

    As you may be aware worpress allows some html in comments including html comments. With wp-super-cache or w3-total-cache installed php code, by design, is allowed in comments too. Not to mention a relatively harmless XSS bug (or feature, depends on who you ask) is elevated into server side code execution.

    The following examples work with wp-super-cache and w3-total-cache, all you need to do is post a comment with:
    – to display time and date:
    <!--mfunc eval(base64_decode( redacted )); --><!--/mfunc-->

    – php shell/backdoor upload:
    <!--mfunc eval(base64_decode( also redacted, see a trend here ;) )); --><!--/mfunc-->
    the shell will eval($_POST[“e”]) and can be accessed at: http://path/to/blog/wp-content/redacted

    I suggest you remove the mfunc/mclude features entirely, it’s way too hazardous to parse and execute the output.

    Regards,
    Csaba

    PS.
    Below you’ll find the same proof of concept examples escaped with base64:

    [ Just plain deleted ]

    http://wordpress.org/extend/plugins/wp-super-cache/

Viewing 15 replies - 1 through 15 (of 19 total)
  • Moderator Jan Dembowski

    @jdembowski

    kisscsaby? Please do not post malware code here on these forums. You could have just as easily made your point without posting a remote shell exploit.

    @kisscsaby; can’t get it to work, but that’s probably due to my lack of appropriate skills 😉

    but isn’t this rather a problem caused by lack of proper sanitizing of comments in wordpress core?

    The moderators censor…redacted the examples I posted earlier, grab this quickly 🙂

    the php version on my server is: <!--mfunc echo PHP_VERSION; --><!--/mfunc-->

    It should work with wp-super-cache and w3-total-cache, both plugins have this mfunc “feature”.
    Allowing html comments in comments probably isn’t very useful but it’s not a vulnerability, the sanitizer’s job is to prevent XSS, seo spam and to allow html by whitelist only. It’s not the sanitizer’s job to remove some random X syntax that has a meaning to random Y and Z plugins.

    Moderator Jan Dembowski

    @jdembowski

    We don’t “censor” exactly, it’s called moderating. And you should NOT have posted that earlier code.

    This one above is fine though. 😉

    the sanitizer’s job is to prevent XSS, seo spam and to allow html by whitelist only.

    Sure, but as html-comments aren’t in the default whitelist (which is “a blockquote code em strong ul ol li”), they should indeed be sanitized, no?

    As nothing much is happening (although I would hope that behind the scene core or w3tc/wp super cache people are working on a solution) and as I indeed have been able to exploit this (although not on all configs, not sure why), I created a little helper plugin to strip mfunc (& related) stuff from comments being posted.

    I’ll probably upload it to the WordPress plugin repository later today.

    @kisscsaby; I suppose you consider this post as a being the pre-disclosure reporting to allow developers to fix? 🙂 When are you planning to disclose?

    @futtta: the “i” modifier is missing from the first regexp and total-cache accepts \s* before “mfunc”. Anyway, the mFUNc feature is broken by design, you can’t trust the output well enough to contain executable code that runs on the server. It’s just a ticking time bomb until someone finds another way to make wordpress output the mfunc syntax, for example the next XSS bug will be a server side code execution vulnerability.
    Well maybe, just maybe if the “mfunc” code has a hmac hash appended that uses a secret unique to the wp installation and if replay attacks are not an issue, you could get away with it, but it’s still a hack. Why complicate it when the most obvious way to deal with this is to remove the “mfunc” feature.

    When are you planning to disclose?

    I’ve already disclosed it 2 weeks ago 🙂 , this is a public forum.

    Thanks for the feedback, I updated the regex accordingly. I’ll leave the discussion on whether this is broken by design to the specialists 😉

    Plugin Author Donncha O Caoimh

    @donncha

    Thanks kisscsaby for finding that bug, but maybe next time email the plugin developers first, please! I’ve just released a new version of WP Super Cache that removes the html comments from user comments. I’ll publish a post about it in a few days time after most people have hopefully upgraded their sites.

    In the next release (1.4) I’m going to disable mfunc and associated functions by default because I suspect most users don’t even use them. Admins will have to enable them on the settings page.

    One more thing / or 2 for that matter, try posting 2 separate comments with:

    comment1: <!--mfunc echo PHP_VERSION; -->
    comment2: <!--/mfunc-->

    not bad, kisscsaby, not bad 🙂

    I’ll tweak wp safer cache to handle that as well (will be not as clean a preg_replace, but well …).

    regarding wp super cache; since 1.3.1 it has mfunc & co disabled by default, so the attach vector is way smaller already.

    Plugin Author Donncha O Caoimh

    @donncha

    kisscsaby – that’s devious! Will it work though as the code in between the open and close PHP tags will have comment html template data too. Does it create a syntax error?

    Plugin Author Donncha O Caoimh

    @donncha

    Can you update your wp-cache.php from http://svn.wp-plugins.org/wp-super-cache/trunk/wp-cache.php ? I modified the regex so it only replaces the leading mfunc etc tags, thus mangling the tag, even though the ending tag will be left over.

    Hi,
    #(<!--\s*(mclude|mfunc|dynamic-cached-content)\s*-+>)#ism
    removes the opening tag only if it’s empty, without code. Why not just zap the entire comment if it looks suspicious until the mfunc feature is removed/rewritten, ex:

    function no_mfunc_in_comments( $comment_data ) {
    
    	if ( is_array( $comment_data ) )
    		$text = &$comment_data[ 'comment_content' ];
    	else
    		$text = &$comment_data;
    
    	if(preg_match('/<!--|-->|<[?%]|[?%]>/', $text) !== 0)
    		$text = 'This is not the comment you are looking for';
    
    	return $comment_data;
    }

    I haven’t had time to test it thoroughly.

    kisscsaby; why would you want to remove comments that match

    <[?%]|[?%]>

    this isn’t part of the mfunc-vulnerability, is it?

Viewing 15 replies - 1 through 15 (of 19 total)
  • The topic ‘mfunc issue’ is closed to new replies.