WordPress.org

Ready to get started?Download WordPress

Forums

WP Super Cache
mfunc issue (20 posts)

  1. kisscsaby
    Member
    Posted 1 year ago #

    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/

  2. 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.

  3. futtta
    Member
    Posted 1 year ago #

    @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?

  4. kisscsaby
    Member
    Posted 1 year ago #

    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.

  5. We don't "censor" exactly, it's called moderating. And you should NOT have posted that earlier code.

    This one above is fine though. ;)

  6. futtta
    Member
    Posted 1 year ago #

    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?

  7. futtta
    Member
    Posted 1 year ago #

    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?

  8. kisscsaby
    Member
    Posted 1 year ago #

    @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.

  9. futtta
    Member
    Posted 1 year ago #

    Thanks for the feedback, I updated the regex accordingly. I'll leave the discussion on whether this is broken by design to the specialists ;-)

  10. Donncha O Caoimh
    Member
    Plugin Author

    Posted 1 year ago #

    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.

  11. kisscsaby
    Member
    Posted 1 year ago #

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

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

  12. futtta
    Member
    Posted 1 year ago #

    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.

  13. Donncha O Caoimh
    Member
    Plugin Author

    Posted 1 year ago #

    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?

  14. Donncha O Caoimh
    Member
    Plugin Author

    Posted 1 year ago #

    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.

  15. kisscsaby
    Member
    Posted 1 year ago #

    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.

  16. futtta
    Member
    Posted 1 year ago #

    kisscsaby; why would you want to remove comments that match

    <[?%]|[?%]>

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

  17. Donncha O Caoimh
    Member
    Plugin Author

    Posted 1 year ago #

    I just updated wp-cache.php so it removes the leading tag and nothing else. The PHP code is left there if it exists. It's nullified now and harmless.

  18. Frederick Townes
    Member
    Posted 1 year ago #

    kisscsaby, a new release with security fixes for W3TC is now available. If you see further issues, please let me know.

  19. futtta
    Member
    Posted 1 year ago #

    Now that both plugins have a version that is not vulnerable, I published a "debrief" about what happened. Feel free to add/ correct info where needed!

  20. Donncha O Caoimh
    Member
    Plugin Author

    Posted 1 year ago #

    And I have completely rewritten the mfunc type functionality. Unfortunately it's not backwards compatible but it is better, and more secure. More details here: http://ocaoimh.ie/2013/04/24/wp-super-cache-1-3-2/#comment-697833

Topic Closed

This topic has been closed to new replies.

About this Plugin

About this Topic