Another round of notable WordPress plugin vulnerabilities made the rounds today- nothing particular noteworthy, just a smattering of XSS, SQLi, and form uploads. The Rich Counter upload vulnerability caught my eye as the PoC notes the exploit vector is through the user agent header. Given that unsanitized user input is more often exploited in query string and POST argument vectors, I thought this was worth a quick once over.
This plugin is awful- it doesn’t conform to WP’s style guides, it abuses the hell out of globals, and it ignores the wpdb object completely, instead using the old mysql PHP API (which means I couldn’t even get it running at first- WordPress will not provide the connection resource that the bare mysql_connect() call needs when PHP 5.5 is running). The noted exploit vector appears to be in this query:
|
1 2 3 4 5 6 |
$query="INSERT INTO rcounter_last SET time='$nowtime', time_last='$nowtime', ip='$g_addr', ref='".mysql_escape_string($counter_ref)."', word='".mysql_escape_string($counter_word)."', browser='$counter_browser', agent='".mysql_escape_string($saschart->SV('HTTP_USER_AGENT'))."', os='$counter_os', res='$counter_res', country='$counter_country', visits=1, details='$details'"; $saschart->mysqlQuery($query); |
For context, here are a few of the helper functions:
|
1 2 3 4 5 6 7 8 9 10 11 12 |
function SV($in_var,$escape=1) { return $escape ? $this->doEscape($_SERVER[$in_var]):$this->delEscape($_SERVER[$in_var]); } [...snip...] function doEscape($str) { return get_magic_quotes_gpc() ? $str:addslashes($str); } function delEscape($str) { return get_magic_quotes_gpc() ? stripslashes($str):$str; } |
So, some attempt is made to sanitize user input; vars that are not escaped in this query are generated via additional function calls, such as “browser=’$counter_browser'”; this is built with the following function:
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 |
function getBrowser() { if (!$this->SV('HTTP_USER_AGENT')) return ""; $arr_browsers = array( "Firefox"=>"Firefox\/([0-9.+]{1,10})", "Opera"=>"opera[ \/]([0-9.]{1,10})", "Internet Explorer"=>"\(compatible; MSIE[ \/]([0-9.]{1,10})", "Netscape"=>"(netscape[0-9]?\/([0-9.]{1,10}))|(^mozilla\/([0-4]\.[0-9.]{1,10}))", "Mozilla"=>"(^mozilla\/[5-9]\.[0-9.]{1,10}.+rv:([0-9a-z.+]{1,10}))|(^mozilla\/([5-9]\.[0-9a-z.]{1,10}))", "Avant Browser"=>"Avant[ ]?Browser", "AOL"=>"(aol[ \/\-]([0-9.]{1,10}))|(aol[ \/\-]?browser)", "Maxthon"=>" Maxthon[\);]", "Safari"=>"safari\/([0-9.]{1,10})", "Crazy Browser"=>"Crazy Browser[ \/]([0-9.]{1,10})", "Konqueror"=>"konqueror\/([0-9.]{1,10})", "Deepnet Explorer"=>"(Deepnet Explorer[\/ ]([0-9.]{1,10}))|( Deepnet Explorer[\);])", ); foreach($arr_browsers as $arr_var=>$arr_val) { if (preg_match("/$arr_val/i",$this->SV('HTTP_USER_AGENT'))) return $arr_var; } return str_replace("'","",preg_replace("/^(\w+).*/","\\1",$this->SV('HTTP_USER_AGENT'))); } |
Two issues so far. We can have this function return any arbitrary string, as long as the user agent header doesn’t match any of the definitions in $arr_browsers- getBrowser() will just return a single quote-stripped first word in our string. The other problem is that the raw user agent header is saved in the “agent” column after being run through mysql_escape_string(), which won’t prevent against stored XSS. Not being able to store JS with quotes might make a cookie slurp a bit more tricky, but not impossible.
Another vulnerability arises in setting the “ip” column (this is done with ip=’$g_addr’). $g_addr is built with another helper function:
|
1 2 3 4 5 6 7 8 9 |
function getIp() { if ($this->SV('HTTP_X_FORWARDED_FOR')) $ip = $this->SV('HTTP_X_FORWARDED_FOR'); elseif ($this->SV('HTTP_CLIENT_IP')) $ip = $this->SV('HTTP_CLIENT_IP'); else $ip = $this->SV('REMOTE_ADDR'); return preg_replace("/,.*/","",$ip); } |
Sure, let’s just go ahead and blindy trust the X-Forwarded-For header. Keep in mind that SV() doesn’t run addslashes if magic_quotes_gpc is enabled- and magic_quotes_gpc doesn’t escape HTTP headers (get/post/cookie, remember?). Additionally, addslashes is known to be an inadequate defense against SQLi.
The Wordfence advisory indicated the plugin may be abandoned, but the changelog notes an update less than 2 months ago. I’d still call this one “spectacularly bad”, if we hadn’t already seen ten thousand examples of awful practices and wet-noodle attempts at sanitation. I’ll go ahead and wrap up with another “helper” function that summarizes how fascinatingly awful this plugin is:
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 |
function decryptText($str_var) { $str_chrs_='e380DVXCBh7ay5ZvnJTstfU6q9IuHNlF1igpPLSGxK2QowWrEd4RzAjkmOcY_bM"/<>= :.'; $str_chrs=strrev($str_chrs_); while ($str_var<>"") { $str_chr=substr($str_var,0,1); $chr_pos=strpos($str_chrs,$str_chr); if ($chr_pos===false) $str_out.=$str_chr; else $str_out.=substr($str_chrs_,$chr_pos,1); $str_var=substr($str_var,1); } return $str_out; } |
… yikes.