| DetailsAffected Software:Ninja Announcements Fixed in Version:1.3 Issue Type:SQL Injection Original Code:Found Here DetailsLots of potential issues here,but we’ll focus on what was patched. Here we have a basic SQL injection vulnerability. The bug is the most simple example of tracing a variable from assignment to usage. On line 54,the $ninja_annc_id is assigned a value directly from the user/attacker controlled $_REQUEST[‘ninja_annc_id’]. The very next line,the developer uses the tainted $ninja_annc_id to string build a SQL statement. The developers addressed this issue by moving the dynamic SQL statement to a prepared SQL statement. Prepared statements are the preferred method for dealing with SQL requests that could potentially contain tainted values. Developers Solution1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
| <?php...snip ... //This if() statement handles user input from the edit section. if($_REQUEST['submitted'] == 'yes'){ // BEGIN submit handling if() $ninja_annc_id = $_REQUEST['ninja_annc_id']; $ninja_annc_begindate = $_REQUEST['begindate']; $ninja_annc_begintimehr = $_REQUEST['begintimehr']; $ninja_annc_begintimemin = $_REQUEST['begintimemin']; $ninja_annc_begintimeampm = $_REQUEST['begintimeampm']; $ninja_annc_enddate = $_REQUEST['enddate']; $ninja_annc_endtimehr = $_REQUEST['endtimehr']; $ninja_annc_endtimemin = $_REQUEST['endtimemin']; $ninja_annc_endtimeampm = $_REQUEST['endtimeampm']; $ninja_annc_begindate = $ninja_annc_begindate.' '.$ninja_annc_begintimehr.':'.$ninja_annc_begintimemin.$ninja_annc_begintimeampm; $ninja_annc_enddate = $ninja_annc_enddate.' '.$ninja_annc_endtimehr.':'.$ninja_annc_endtimemin.$ninja_annc_endtimeampm; //echo "begin before ".$ninja_annc_begindate; //echo "end before ".$ninja_annc_enddate; $ninja_annc_message = stripslashes($_REQUEST['content']); $ninja_annc_active = 1; $ninja_annc_location = $_REQUEST['ninja_annc_location']; $ninja_annc_begindate = strtotime($ninja_annc_begindate); $ninja_annc_enddate = strtotime($ninja_annc_enddate); //echo "begin after ".$ninja_annc_begindate; //echo "end after ".$ninja_annc_enddate; if($_REQUEST['ignoredates'] == "checked"){ $ninja_annc_begindate = 0; $ninja_annc_enddate = 0; } if($_REQUEST['ninja_annc_id'] == 'new'){ $rows_affected = $wpdb->insert( $ninja_annc_table_name, array( 'begindate' => $ninja_annc_begindate, 'enddate' => $ninja_annc_enddate, 'message' => $ninja_annc_message, 'active' => '0', 'location' => $ninja_annc_location ) ); }else{ $wpdb->update( $ninja_annc_table_name, array( 'begindate' => $ninja_annc_begindate, 'enddate' => $ninja_annc_enddate, 'message' => $ninja_annc_message, 'location' => $ninja_annc_location ), array( 'id' => $ninja_annc_id )); } echo "<script language='javascript'>window.location = '".$admin_url."'</script>"; } // END submit handling if() //This if...else() statement handles the nuts and bolts of our html output. //Eventually it will be replaced by a switch(). //Flow goes:Edit Announcement? ->New Announcement? ->Table. //This part of our If...else statement creates the editing HTML if($_REQUEST['action'] == 'edit') { //BEGIN edit handling if() $ninja_annc_id = $_REQUEST['ninja_annc_id'];- $ninja_annc_row = $wpdb->get_row("SELECT * FROM $ninja_annc_table_name WHERE id = $ninja_annc_id", ARRAY_A );+ $ninja_annc_row = $wpdb->get_row( + $wpdb->prepare( "SELECT * FROM $ninja_annc_table_name WHERE id = %d", $ninja_annc_id), ARRAY_A ); $ninja_annc_id = $ninja_annc_row['id']; $ninja_annc_location = $ninja_annc_row['location']; $ninja_annc_message = stripslashes($ninja_annc_row['message']); $ninja_annc_begin = $ninja_annc_row['begindate']; $ninja_annc_end = $ninja_annc_row['enddate']; $rightnow = current_time ("timestamp"); if($ninja_annc_end != 0){ $ninja_annc_begindate = date("m/d/Y", $ninja_annc_begin); $ninja_annc_begintimehr = date("g", $ninja_annc_begin); $ninja_annc_begintimemin = date("i", $ninja_annc_begin); $ninja_annc_begintimeampm = date("a", $ninja_annc_begin); $ninja_annc_enddate = date("m/d/Y", $ninja_annc_end); $ninja_annc_endtimehr = date("g", $ninja_annc_end); $ninja_annc_endtimemin = date("i", $ninja_annc_end); $ninja_annc_endtimeampm = date("a", $ninja_annc_end); }else{ $ninja_annc_ignore = 1; $ninja_annc_begindate = date("m/d/Y", $rightnow); $ninja_annc_enddate = date("m/d/Y", $rightnow); } //$ninja_annc_begindate = $ninja_annc_begindate.' '.$ninja_annc_begintimehr.':'.$ninja_annc_begintimemin.$ninja_annc_begintimeampm; //echo $ninja_annc_begindate; wp_tiny_mce ( false, // true makes the editor "teeny" array( "theme_advanced_path" => false ) ); wp_tiny_mce_preload_dialogs (); ?> <div class="wrap"> <div id="ninja_annc_options_edit"class="icon32"><br></div> <h2 id="opener">Edit Announcement - ID: <?php echo $ninja_annc_id;?></h2> <form name=""action=""method="post"> <input type="hidden"name="submitted"value="yes"> <input type="hidden"name="ninja_annc_id"value=" <?php echo $ninja_annc_id;?>"> Ignore Dates:<input type="checkbox"name="ignoredates"id="ignoredates"value="checked" <?php if($ninja_annc_ignore == 1){ echo "checked";}?>><br> Begin Date:<input type="text"class="date"name="begindate"id="begindate"value=" <?php echo $ninja_annc_begindate;?>" <?php if($ninja_annc_ignore == 1){ echo "style='background-color:gray' disabled";}?>> Time: <select name="begintimehr"id="begintimehr"class="time" <?php if($ninja_annc_ignore == 1){ echo "style='background-color:gray' disabled";}?>> <?php $x = 1; while($x <= 12){ echo "<option"; if($x <= 9){ echo "value = '0$x'"; }else{ echo "value = '$x'"; } if($ninja_annc_begintimehr == $x){ echo "selected"; }elseif($x == 12 && $ninja_annc_ignore == 1){ echo "selected"; } echo ">$x</option>"; $x++; } ?>...snip... ?> |
DetailsAffected Software:PixelPost Fixed in Version:? Issue Type:Insecure password reset functionality Original Code:Found Here DetailsThis week’s bug is more of a design issue as opposed to an implementation issue. I actually first heard about this code from SkullSecurity’s excellent articles on “Hacking Crappy Password Resets” articles published in late March. SkullSecurity does an excellent job of explaining that line 31(the line that does the actual password generation) is full of bad security design. First,the password reset code is using the MD5() function in PHP. MD5() takes a string and returns a MD5 hash of that string. In this password reset code,we see that we are hashing the value of:‘time’. rand(1,16000). SkullSecurity astutely points out that the password reset code is using the string ‘time’,NOT the function time() which would return the number of seconds since epoch. The string ‘time’ has a random number between 1 and 16000 appended to it and is passed to MD5(). Assuming rand() generates a truly random number between 1 and 16000,we only have 16000 possible combinations for the newly reset password. Unfortunately,in many circumstances,rand() doesn’t generate a truly random number. Many times,rand() will use system time as a seed value and produce a number that is predictable. In many cases,it may be possible to predict or leak system time for the machine generating the random password. Stefan Esser wrote an excellent article on the challenges of generating random numbers in PHP which covers many of these scenarios. So what should the author do? Change ‘time’ to time()? Even if the author changed the string ‘time’ to the function time(),time() in PHP only returns seconds since epoch time. If an attacker can leak system time,they’ll still be able to predict the password reset value… Developers Solution<?php...snip...// forgot password?if(isset($_GET['x']) &&$_GET['x']=='passreminder'){echo '<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN""http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"><html><head><title>$admin_lang_pw_title</title><meta http-equiv="Content-Type"content="text/html;charset=utf-8"/></head><body><p style="border:solid 2px;padding:5px;color:red;font-weight:bold;font-size:11px;margin-left:auto;margin-right:auto;margin-top:10%;font-family:verdana,arial,sans-serif;text-align:center;">';if ($cfgrow['admin']!= $_POST['user']){echo "<span class=\"confirm\">$admin_lang_pw_wronguser</span><br />";echo "<br /><a href='index.php'>$admin_lang_pw_back</a></body></html>";die()}if ($cfgrow['email']== ""){echo "<span class=\"confirm\">$admin_lang_pw_noemail</span><br />";echo "<br /><a href='index.php' >$admin_lang_pw_back </a></body></html>";die()}if (strtolower($cfgrow['email'])==strtolower($_POST['reminderemail'])){// generate a random new pass$user_pass = substr( MD5('time' . rand(1,16000)),0,6);$query = "update ".$pixelpost_db_prefix."config set password=MD5('$user_pass') where admin='".$cfgrow['admin']."'";if(mysql_query($query)){$subject = "$admin_lang_pw_subject";$body = "$admin_lang_pw_text_1 \n\n";$body .= "$admin_lang_pw_usertext ".$cfgrow['admin']."\n";$body .= "$admin_lang_pw_mailtext ".$cfgrow['email']."\n\n";$body .= "$admin_lang_pw_newpw $user_pass";$body .= "\n\n$admin_lang_pw_text_7".$cfgrow['siteurl']."admin $admin_lang_pw_text_8";$headers = "Content-type:text/plain;charset=UTF-8\n";$headers .= "$admin_lang_pw_text_2 <".$cfgrow['email'].">\n";$recipient_email = $cfgrow['email'];if (mail($recipient_email,$subject,$body,$headers)){echo "$admin_lang_pw_text_3".$cfgrow['email']}else{echo "$admin_lang_pw_text_3"}echo "<br /><a href='index.php' >$admin_lang_pw_back </a></body></html>";die()}else{$dberror = mysql_error();echo "$admin_lang_pw_text_5 ".$dberror ."$admin_lang_pw_text_5 ";echo "<br /><a href='index.php' >$admin_lang_pw_back </a></body></html>";die()}}else{echo "<span class=\"confirm\">$admin_lang_pw_notsent</span><br />";echo "<br /><a href='index.php'>$admin_lang_pw_back </a></body></html>";die()}// end else (strtolower($cfgrow['email'])==strtolower($_POST['reminderemail']) &$cfgrow['email']!= "")} // end if($_GET['x']=='passreminder')?>DetailsAffected Software:WordPress Core Fixed in Version:2.8 Issue Type:Cross Site Scripting Original Code:Found Here DetailsThis week’s bug was subtle. The patch submitted by the developer addresses an XSS bug. Looking at the diff,we see that $title and $selection come from the query string. These values are fixed up before being assigned to a variable. The developers changed the way $title is assigned in the diff. It’s difficult to see why $title needs to be changed,so we’ll ignore that change for now. $selection gives some hints towards XSS. $selection is assigned from the query string value $_GET[‘s’] which is sent through the trim() and aposfix() functions. Immediately following the $selection variable assignment we see the $selection being manipulated with some HTML tags. This is a good indication that $selection will eventually be used to build HTML markup. I usually recommend developers encode/sanitize values as close to the point of consumption as possible,but this case is different. $selection contains HTML tags (the <p>and </p>tags),so encoding the whole value at the point of consumption will also encode those tags. The <p>tags are probably designed to be rendered by the browser. Due to the mixing of HTML tags and regular data,the developer will have to encode the user controlled values during variable assignment. This is exactly what the WordPress developers did in the patch. Developers Solution<?phprequire_once('admin.php');header('Content-Type:' . get_option('html_type') . ';charset=' . get_option('blog_charset'));if ( ! current_user_can('edit_posts') )wp_die( __( 'Cheatin’uh?' ) );function aposfix($text){$translation_table[chr(34)] = '"';$translation_table[chr(38)] = '&';$translation_table[chr(39)] = ''';return preg_replace("/&(?![A-Za-z]{0,4}\w{2,3};|#[0-9]{2,3};)/","&",strtr($text,$translation_table))}function press_it(){// define some basic variables$quick['post_status'] = 'draft';// set as draft first$quick['post_category'] = isset($_REQUEST['post_category']) ? $_REQUEST['post_category']:null;$quick['tax_input'] = isset($_REQUEST['tax_input']) ? $_REQUEST['tax_input']:'';$quick['post_title'] = isset($_REQUEST['title']) ? $_REQUEST['title']:'';$quick['post_content'] = '';// insert the post with nothing in it,to get an ID$post_ID = wp_insert_post($quick,true);$content = isset($_REQUEST['content']) ? $_REQUEST['content']:'';$upload = false;if( !empty($_REQUEST['photo_src']) &¤t_user_can('upload_files') )foreach( (array) $_REQUEST['photo_src'] as $key =>$image)// see if files exist in content - we don't want to upload non-used selected files.if( strpos($_REQUEST['content'],$image) !== false ){$desc = isset($_REQUEST['photo_description'][$key]) ? $_REQUEST['photo_description'][$key]:'';$upload = media_sideload_image($image,$post_ID,$desc);// Replace the POSTED content <img>with correct uploaded ones. Regex contains fix for Magic Quotesif( !is_wp_error($upload) ) $content = preg_replace('/<img ([^>]*)src=\\\?(\"|\')'.preg_quote($image,'/').'\\\?(\2)([^>\/]*)\$http_status_codes=array( 100 =>'Continue', 101 =>'Switching Protocols', 102 =>'Processing', 200 =>'OK', 201 =>'Created', 202 =>'Accepted', 203 =>'Non-Authoritative Information', 204 =>'No Content', 205 =>'Reset Content', 206 =>'Partial Content', 207 =>'Multi-Status', 300 =>'Multiple Choices', 301 =>'Moved Permanently', 302 =>'Found', 303 =>'See Other', 304 =>'Not Modified', 305 =>'Use Proxy', 306 =>'unused', 307 =>'Temporary Redirect', 400 =>'Bad Request', 401 =>'Authorization Required', 402 =>'Payment Required', 403 =>'Forbidden', 404 =>'Not Found', 405 =>'Method Not Allowed', 406 =>'Not Acceptable', 407 =>'Proxy Authentication Required', 408 =>'Request Time-out', 409 =>'Conflict', 410 =>'Gone', 411 =>'Length Required', 412 =>'Precondition Failed', 413 =>'Request Entity Too Large', 414 =>'Request-URI Too Large', 415 =>'Unsupported Media Type', 416 =>'Requested Range Not Satisfiable', 417 =>'Expectation Failed', 418 =>'unused', 419 =>'unused', 420 =>'unused', 421 =>'unused', 422 =>'Unprocessable Entity', 423 =>'Locked', 424 =>'Failed Dependency', 425 =>'No code', 426 =>'Upgrade Required', 500 =>'Internal Server Error', 501 =>'Method Not Implemented', 502 =>'Bad Gateway', 503 =>'Service Temporarily Unavailable', 504 =>'Gateway Time-out', 505 =>'HTTP Version Not Supported', 506 =>'Variant Also Negotiates', 507 =>'Insufficient Storage', 508 =>'unused', 509 =>'unused', 510 =>'Not Extended',);$err_status_codes = array('100'=>array('Continue',''),'101'=>array('Switching Protocols',''),'102'=>array('Processing',''),'200'=>array('OK',''),'201'=>array('Created',''),'202'=>array('Accepted',''),'203'=>array('Non-Authoritative Information',''),'204'=>array('No Content',''),'205'=>array('Reset Content',''),'206'=>array('Partial Content',''),'207'=>array('Multi-Status',''),'300'=>array('Multiple Choices',''),'301'=>array('Moved Permanently','The document has moved <a href="THEREQUESTURI">here</a>.'),'302'=>array('Found','The document has moved <a href="THEREQUESTURI">here</a>.'),'303'=>array('See Other','The answer to your request is located <a href="THEREQUESTURI">here</a>.'),'304'=>array('Not Modified',''),'305'=>array('Use Proxy','This resource is only accessible through the proxy THEREQUESTURIYou will need to configure your client to use that proxy.'),'307'=>array('Temporary Redirect','The document has moved <a href="THEREQUESTURI">here</a>.'),'400' =>array('Bad Request','Your browser sent a request that this server could not understand.'),'401' =>array('Authorization Required','This server could not verify that you are authorized to access the document requested. Either you supplied the wrong credentials (e.g.,bad password),or your browser doesn\'t understand how to supply the credentials required.'),'402' =>array('Payment Required','INTERROR'),'403' =>array('Forbidden','You don\'t have permission to access THEREQUESTURI on this server.'),'404' =>array('Not Found','We couldn\'t find <acronym title="THEREQUESTURI">that uri</acronym>on our server,though it\'s most certainly not your fault.'),'405' =>array('Method Not Allowed','The requested method THEREQMETH is not allowed for the URL THEREQUESTURI.'),'406' =>array('Not Acceptable','An appropriate representation of the requested resource THEREQUESTURI could not be found on this server.'),'407' =>array('Proxy Authentication Required','This server could not verify that you are authorized to access the document requested. Either you supplied the wrong credentials (e.g.,bad password),or your browser doesn\'t understand how to supply the credentials required.'),'408' =>array('Request Time-out','Server timeout waiting for the HTTP request from the client.'),'409' =>array('Conflict','INTERROR'),'410' =>array('Gone','The requested resourceTHEREQUESTURIis no longer available on this server and there is no forwarding address. Please remove all references to this resource.'),'411' =>array('Length Required','A request of the requested method GET requires a valid Content-length.'),'412' =>array('Precondition Failed','The precondition on the request for the URL THEREQUESTURI evaluated to false.'),'413' =>array('Request Entity Too Large','The requested resource THEREQUESTURI does not allow request data with GET requests,or the amount of data provided in the request exceeds the capacity limit.'),'414' =>array('Request-URI Too Large','The requested URL\'s length exceeds the capacity limit for this server.'),'415' =>array('Unsupported Media Type','The supplied request data is not in a format acceptable for processing by this resource.'),'416' =>array('Requested Range Not Satisfiable',''),'417' =>array('Expectation Failed','The expectation given in the Expect request-header field could not be met by this server. The client sent <code>Expect:</code>'),'422' =>array('Unprocessable Entity','The server understands the media type of the request entity,but was unable to process the contained instructions.'),'423' =>array('Locked','The requested resource is currently locked. The lock must be released or proper identification given before the method can be applied.'),'424' =>array('Failed Dependency','The method could not be performed on the resource because the requested action depended on another action and that other action failed.'),'425' =>array('No code','INTERROR'),'426' =>array('Upgrade Required','The requested resource can only be retrieved using SSL. The server is willing to upgrade the current connection to SSL,but your client doesn\'t support it. Either upgrade your client,or try requesting the page using https://'),'500' =>array('Internal Server Error','INTERROR'),'501' =>array('Method Not Implemented','GET to THEREQUESTURI not supported.'),'502' =>array('Bad Gateway','The proxy server received an invalid response from an upstream server.'),'503' =>array('Service Temporarily Unavailable','The server is temporarily unable to service your request due to maintenance downtime or capacity problems. Please try again later.'),'504' =>array('Gateway Time-out','The proxy server did not receive a timely response from the upstream server.'),'505' =>array('HTTP Version Not Supported','INTERROR'),'506' =>array('Variant Also Negotiates','A variant for the requested resource <code>THEREQUESTURI</code>is itself a negotiable resource. This indicates a configuration error.'),'507' =>array('Insufficient Storage','The method could not be performed on the resource because the server is unable to store the representation needed to successfully complete the request. There is insufficient free space left in your storage allocation.'),'510' =>array('Not Extended','A mandatory extension policy in the request is not accepted by the server for this resource.'));if (isset($_SERVER['REDIRECT_STATUS'])) $err_code = $_SERVER['REDIRECT_STATUS'];$err_req_meth = $_SERVER['REQUEST_METHOD'];$err_req = htmlentities(strip_tags($_SERVER['REQUEST_URI']));$err_phrase = $err_status_codes[$err_code][0];$err_body = str_replace( array('INTERROR','THEREQUESTURI','THEREQMETH'),array('The server encountered an internal error or misconfiguration and was unable to complete your request.',$err_req,$err_req_meth),$err_status_codes[$err_code][1]);@header("HTTP/1.1 $err_code $err_phrase",1);@header("Status:$err_code $err_phrase",1);if ( $err_code=='400'||$err_code=='403'||$err_code=='405'||$err_code[0]=='5'){@header("Connection:close",1);if ($err_code == '405') @header('Allow:GET,HEAD,POST,OPTIONS,TRACE');echo "<!DOCTYPE HTML PUBLIC \"-//IETF//DTD HTML 2.0//EN\">\n<html>\n<head>\n<title>{$err_code}{$err_phrase}</title>\n<h1>{$err_phrase}</h1>\n<p>{$err_body}<br>\n</p>\n</body></html>"} else echo '<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"><html xml:lang="en"lang="en"><head><title>'.$err_code.' '.$err_phrase.'</title><meta http-equiv="content-type"content="text/html;charset=UTF-8"/></head><body><h1>'.$err_code.' '.$err_phrase.'</h1><hr /><p>'.$err_body.'<br /></p>-<pre>-'.print_r($_SERVER,1).'-</pre></body></html>';?>DetailsAffected Software:WordPress Core Fixed in Version:2.2-alpha Issue Type:Cross Site Scripting Original Code:Found Here DetailsA couple of bugs affecting WordPress core here. On line 73,we see that $_SERVER['REQUEST_URI'] is passed to add_query_arg(). From the provided code sample,it’s difficult to see that this results in XSS. The developers addressed this by encoding the return value from add_query_arg(). The second issue starts at wp_get_referer(). This function checks $_REQUEST['_wp_http_referer'] and/or $_SERVER['HTTP_REFERER'] for possible values. This makes is so the attacker has several options for tainting wp_get_referer(). Passing a tainted GET or POST parameter will do the trick. Simply redirecting to the vulnerable WordPress installation from a tainted URL will also taint the value returned from wp_get_referer().
Later,in wp_nonce_ays(),the tainted wp_get_referer() value is passed to the $adminurl variable. $adminurl is then used to build HTML markup in a couple of different places. The WP developers addressed this issue by passing wp_get_referer() before assinging that value to $adminurl. Seems like an effective fix? Well,url values are always a bit tricky. These values have to be encoded before being used to build HTML markup. URLs also have to be validated if they are being passed to a SRC,HREF,or other HTML attribute which causes a browser navigation or request. In this example,the tainted $adminurl is used to populate an A HREF value. The value is encoded,however that encoding doesn’t stop an attacker from passing a javascript:payload url… Developers Solution<?php...snip...function wp_original_referer_field(){echo '<input type="hidden"name="_wp_original_http_referer"value="' . attribute_escape(stripslashes($_SERVER['REQUEST_URI'])) . '"/>'}function wp_get_referer(){foreach ( array($_REQUEST['_wp_http_referer'],$_SERVER['HTTP_REFERER']) as $ref )if ( !empty($ref) )return $ref;return false}function wp_get_original_referer(){if ( !empty($_REQUEST['_wp_original_http_referer']) )return $_REQUEST['_wp_original_http_referer'];return false}function wp_mkdir_p($target){// from php.net/mkdir user contributed notesif (file_exists($target)){if (! @ is_dir($target))return false;elsereturn true}// Attempting to create the directory may clutter up our display.if (@ mkdir($target)){$stat = @ stat(dirname($target));$dir_perms = $stat['mode'] &0007777;// Get the permission bits.@ chmod($target,$dir_perms);return true} else{if ( is_dir(dirname($target)) )return false}// If the above failed,attempt to create the parent node,then try again.if (wp_mkdir_p(dirname($target)))return wp_mkdir_p($target);return false}...snip...function wp_nonce_ays($action){global $pagenow,$menu,$submenu,$parent_file,$submenu_file;$adminurl = get_option('siteurl') . '/wp-admin';if ( wp_get_referer() )-$adminurl = wp_get_referer();+$adminurl = attribute_escape(wp_get_referer());$title = __('WordPress Confirmation');// Remove extra layer of slashes.$_POST = stripslashes_deep($_POST );if ( $_POST ){$q = http_build_query($_POST);$q = explode( ini_get('arg_separator.output'),$q);$html .= "\t<form method='post' action='$pagenow'>\n";foreach ( (array) $q as $a ){$v = substr(strstr($a,'='),1);$k = substr($a,0,-(strlen($v)+1));$html .= "\t\t<input type='hidden' name='". attribute_escape(urldecode($k)) . "' value='". attribute_escape(urldecode($v)) . "' />\n"}$html .= "\t\t<input type='hidden' name='_wpnonce' value='". wp_create_nonce($action) . "' />\n";$html .= "\t\t<div id='message' class='confirm fade'>\n\t\t<p>". wp_specialchars(wp_explain_nonce($action)) . "</p>\n\t\t<p><a href='$adminurl'>". __('No') . "</a><input type='submit' value='". __('Yes') . "' /></p>\n\t\t</div>\n\t</form>\n"} else{-$html .= "\t<div id='message' class='confirm fade'>\n\t<p>". wp_specialchars(wp_explain_nonce($action)) . "</p>\n\t<p><a href='$adminurl'>". __('No') . "</a><a href='". add_query_arg( '_wpnonce',wp_create_nonce($action),$_SERVER['REQUEST_URI'] ) . "'>". __('Yes') . "</a></p>\n\t</div>\n";+ $html .= "\t<div id='message' class='confirm fade'>\n\t<p>". wp_specialchars(wp_explain_nonce($action)) . "</p>\n\t<p><a href='$adminurl'>". __('No') . "</a><a href='". attribute_escape(add_query_arg( '_wpnonce',wp_create_nonce($action),$_SERVER['REQUEST_URI'] )) . "'>". __('Yes') . "</a></p>\n\t</div>\n"}$html .= "</body>\n</html>";wp_die($html,$title)}function wp_die($message,$title = ''){global $wp_locale;header('Content-Type:text/html;charset=utf-8');if ( empty($title) )$title = __('WordPress ›Error');if ( strstr($_SERVER['PHP_SELF'],'wp-admin') )$admin_dir = '';else$admin_dir = 'wp-admin/';?><!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN""http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"><html xmlns="http://www.w3.org/1999/xhtml"<?php if ( function_exists('language_attributes') ) language_attributes();?>><head><title><?php echo $title ?></title><meta http-equiv="Content-Type"content="text/html;charset=utf-8"/><link rel="stylesheet"href="<?php echo $admin_dir;?>install.css"type="text/css"/><?php if ( ('rtl' == $wp_locale->text_direction) ):?><link rel="stylesheet"href="<?php echo $admin_dir;?>install-rtl.css"type="text/css"/><?php endif;?></head><body><h1 id="logo"><img alt="WordPress"src="<?php echo $admin_dir;?>images/wordpress-logo.png"/></h1><p><?php echo $message;?></p></body></html>...snip...DetailsAffected Software:Cubed Fixed in Version:1.0 RC2 Issue Type:Cross Site Scripting Original Code:Found Here DetailsThis week’s patch is a good one. The code sample was basically a library that only contained functions. While there isn’t a blatant vulnerability in the library,there is a startling function called “PrepDataForScript”. Looking at PrepDataForScript,it’s obvious this function is meant to provide some sanitization. Unfortunately,the routine isn’t very robust. When you see things like the code snippet below,you know the developer is headed in the wrong direction: $strData = str_replace(“","</script>",$strData); Fortunately,the Cubed developers were smart enough to realize that this function is dangerous and will probably lead to a false sense of security. Instead of trying to fix it up,they just removed the function entirely. Developers Solution<?php...snip...function QcodoHandleError($__exc_errno,$__exc_errstr,$__exc_errfile,$__exc_errline,$blnExit = true){// If a command is called with "@",then we should returnif (error_reporting() == 0)return;if (class_exists('QApplicationBase'))QApplicationBase::$ErrorFlag = true;global $__exc_strType;if (isset($__exc_strType))return;$__exc_strType = "Error";$__exc_strMessage = $__exc_errstr;switch ($__exc_errno){case E_ERROR:$__exc_strObjectType = "E_ERROR";break;case E_WARNING:$__exc_strObjectType = "E_WARNING";break;case E_PARSE:$__exc_strObjectType = "E_PARSE";break;case E_NOTICE:$__exc_strObjectType = "E_NOTICE";break;case E_STRICT:$__exc_strObjectType = "E_STRICT";break;case E_CORE_ERROR:$__exc_strObjectType = "E_CORE_ERROR";break;case E_CORE_WARNING:$__exc_strObjectType = "E_CORE_WARNING";break;case E_COMPILE_ERROR:$__exc_strObjectType = "E_COMPILE_ERROR";break;case E_COMPILE_WARNING:$__exc_strObjectType = "E_COMPILE_WARNING";break;case E_USER_ERROR:$__exc_strObjectType = "E_USER_ERROR";break;case E_USER_WARNING:$__exc_strObjectType = "E_USER_WARNING";break;case E_USER_NOTICE:$__exc_strObjectType = "E_USER_NOTICE";break;default:$__exc_strObjectType = "Unknown";break}$__exc_strFilename = $__exc_errfile;$__exc_intLineNumber = $__exc_errline;$__exc_strStackTrace = "";$__exc_objBacktrace = debug_backtrace();for ($__exc_intIndex = 0;$__exc_intIndex <count($__exc_objBacktrace);$__exc_intIndex++){$__exc_objItem = $__exc_objBacktrace[$__exc_intIndex];$__exc_strKeyFile = (array_key_exists("file",$__exc_objItem)) ? $__exc_objItem["file"]:"";$__exc_strKeyLine = (array_key_exists("line",$__exc_objItem)) ? $__exc_objItem["line"]:"";$__exc_strKeyClass = (array_key_exists("class",$__exc_objItem)) ? $__exc_objItem["class"]:"";$__exc_strKeyType = (array_key_exists("type",$__exc_objItem)) ? $__exc_objItem["type"]:"";$__exc_strKeyFunction = (array_key_exists("function",$__exc_objItem)) ? $__exc_objItem["function"]:"";$__exc_strStackTrace .= sprintf("#%s %s(%s):%s%s%s()\n",$__exc_intIndex,$__exc_strKeyFile,$__exc_strKeyLine,$__exc_strKeyClass,$__exc_strKeyType,$__exc_strKeyFunction)}if (ob_get_length()){$__exc_strRenderedPage = ob_get_contents();ob_clean()}// Call to display the Error Page (as defined in configuration.inc.php)require(__DOCROOT__ . ERROR_PAGE_PATH);if($blnExit)exit}-function PrepDataForScript($strData){-$strData = str_replace("\\","\\\\",$strData);-$strData = str_replace("\n","\\n",$strData);-$strData = str_replace("\r","\\r",$strData);-$strData = str_replace("\"",""",$strData);-$strData = str_replace("</script>","</script>",$strData);-$strData = str_replace("</Script>","</script>",$strData);-$strData = str_replace("</SCRIPT>","</script>",$strData);-return $strData;-}?>DetailsAffected Software:StatPressCN Fixed in Version:1.9.1 Issue Type:SQL Injection Original Code:Found Here DetailsThis patch was full of interesting tidbits. First,the change log for this patch is as follows: **1.9.1**
+ fix a flaw allowing a remote cross-site scripting attack
Keep the change list description in mind as we go over the patch submitted by the developers. The submitted patch is pretty simple. There is an additional qualifier set for an if statement that checks to see if $_GET["where$i"] is contained within array $f. It’s difficult to determine whether this is true… but it doesn’t really matter. The second change is an addslashes to $_GET["what$i"] before using the tainted query string parameter to build a dynamic SQL statement. This is to prevent an obvious SQL injection bug in the LIKE operator of the SQL statement. What’s surprising is the developer missed the $_GET["where$i"] query string parameter used to build the SQL statement on the same line. This bug is equally devastating and results in SQL injection against the application. So despite the change log description,this patch is to address a SQL injection bug,NOT an XSS. Looking through the rest of the code,we see XSS (lines 7-9 and 17) and SQL injection bugs (lines 57,65,77) littered throughout the code base. These bugs still exist in the latest version,are not patched,and put users at risk. If you have this plug-in installed,your server and users are at significant risk! Developers Solution </table> <br> <table> <tr><td><table><tr><td><input type=checkbox name=oderbycount value=checked <?php print $_GET['oderbycount'] ?>><?php _e('sort by count if grouped','statpresscn');?></td></tr><tr><td><input type=checkbox name=spider value=checked <?php print $_GET['spider'] ?>><?php _e('include spiders/crawlers/bot','statpresscn');?></td></tr><tr><td><input type=checkbox name=feed value=checked <?php print $_GET['feed'] ?>><?php _e('include feed','statpresscn');?></td></tr></table></td><td width=15></td><td><table><tr> <td><?php _e('Limit results to','statpresscn');?> <select name=limitquery><?php if($_GET['limitquery'] >0){print "<option>".$_GET['limitquery']."</option>"} ?><option>200</option><option>150</option><option>50</option></select> </td></tr><tr><td> </td></tr><tr> <td align=right><input type=submit value=<?php _e('Search','statpresscn');?>name=searchsubmit></td></tr></table></td> </tr> </table><!-- It's strange that the page value should be spc-search,and not others. --> <input type=hidden name=page value='spc-search'><input type=hidden name=statpress_action value=search> </form><br> <?php if(isset($_GET['searchsubmit'])){ # query builder $qry=""; # FIELDS $fields=""; for($i=1;$i<=5;$i++){- if($_GET["where$i"] != ''){+if($_GET["where$i"] != '' &&array_key_exists($_GET["where$i"],$f)){//??where?????? $fields.=$_GET["where$i"].",";} } $fields=rtrim($fields,","); # WHERE $where="WHERE 1=1"; if($_GET['spider'] != 'checked'){$where.="AND spider=''";} if($_GET['feed'] != 'checked'){$where.="AND feed=''";} for($i=1;$i<=5;$i++){if(($_GET["what$i"] != '') &&($_GET["where$i"] != '')){- $where.="AND ".$_GET["where$i"]."LIKE '%".$_GET["what$i"]."%'";+$where.="AND ".$_GET["where$i"]."LIKE '%".addslashes($_GET["what$i"])."%'";//addslashes?????? } } # ORDER BY $orderby=""; for($i=1;$i<=5;$i++){if(($_GET["sortby$i"] == 'checked') &&($_GET["where$i"] != '')){$orderby.=$_GET["where$i"].',';} } # GROUP BY $groupby=""; for($i=1;$i<=5;$i++){if(($_GET["groupby$i"] == 'checked') &&($_GET["where$i"] != '')){$groupby.=$_GET["where$i"].',';} } if($groupby != ''){$grouparray = explode(",",rtrim($groupby,','));$groupby="GROUP BY ".rtrim($groupby,',');$fields.=",count(*) as totale";if($_GET['oderbycount'] == 'checked'){$orderby="totale DESC,".$orderby;} } if($orderby != ''){$orderby="ORDER BY ".rtrim($orderby,',');} $limit="LIMIT ".$_GET['limitquery']; # Results print "<h2>".__('Results','statpresscn')."</h2>"; $sql="SELECT $fields FROM $table_name $where $groupby $orderby $limit;"; //print "$sql<br>"; print "<table class='widefat'><thead><tr>"; for($i=1;$i<=5;$i++){if($_GET["where$i"] != ''){print "<th scope='col'>";if((count($grouparray)>0)&&in_array($_GET["where$i"],$grouparray)){print "<font color=red>";} print ucfirst($f[$_GET["where$i"]]);if((count($grouparray)>0)&&in_array($_GET["where$i"],$grouparray)){print "</font>";} print "</th>";} } if($groupby != ''){print "<th scope='col'><font color=red>".__('Count','statpresscn')."</font></th>";} print "</tr></thead><tbody id='the-list'>"; $qry=$wpdb->get_results($sql,ARRAY_N); $cloumnscount = count($wpdb->get_col_info("name")); foreach ($qry as $rk){print "<tr>";for($i=1;$i<=$cloumnscount;$i++){print "<td>";if($_GET["where$i"] == 'urlrequested'){print "<a href=".heart5_config_url($rk[$i-1])."target=_heart5>";print iri_StatPress_Decode($rk[$i-1]);print "</a>";} else{print $rk[$i-1];}// print $rk[$i-1];print "</td>";} print "</tr>"; } print "</table>"; print "<br /><br /><font size=1 color=gray>sql:$sql</font>"; }?></div>DetailsAffected Software:Lazyest-Gallery Fixed in Version:0.9 Issue Type:Cross Site Scripting (XSS) Original Code:Found Here DetailsFor most security issues,I give the developer the benefit of the doubt. It’s tough to keep track of all the corner cases and security nuances. For this diff however,there is no excuse. First,let’s cover what the patch fixes. On line 18,the developer was taking a tainted value passed via query string parameter and using that value to build HTML markup. This is XSS in its most classic form. Also,on line 58 the same tainted input is used to build the SRC attribute for an image tag,also resulting in XSS. The developer chose to encode both of these tainted values before using them in the HTML output. Now,let’s talk about the problems with this patch. First,the tainted value used to build the SRC attribute for an image tag needs additional validation. SRC attributes are tricky as they usually cause the browser to issue a request. Escaping the tainted SRC value only prevents the attacker from breaking out of the attribute and injecting their own HTML. Escaping doesn’t prevent the attacker from passing a well formed URI like javascript:javascript-payload-here. I can let the developer slide on this one… chalk it up as a lesson on corner cases. Now,if you look at the patched line,you’ll see that the ALT attribute for the same image tag also contains a XSS vulnerability. Yes,the developer missed a XSS vulnerability that is less than 5 characters away from a fixed XSS vulnerability. This also shows that the developer never tested the patch. The tainted query string parameter is the same for all the vulnerable sections. If the developer tried to test this patch,they would have discovered they were still exposed… Developers Solution<?php// Don't remove this lines:require_once('../../../wp-blog-header.php');global $lg_gallery;?><!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN""http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd"><html xmlns="http://www.w3.org/1999/xhtml"><head><meta http-equiv="Content-Type"content="<?php bloginfo('html_type');?>;charset=<?php bloginfo('charset');?>"/><meta name="generator"content="WordPress <?php bloginfo('version');?>"/>-<title><?php echo $_GET['image'] ?></title>+<title><?php echo esc_html($_GET['image']) ?></title><style type="text/css">body{text-align:center;margin:0;padding:0}img{border:none}</style><script type="text/javascript">function WinWidth(){if (window.innerWidth!=window.undefined) return window.innerWidth;if (document.compatMode=='CSS1Compat') return document.documentElement.clientWidth;if (document.body) return document.body.clientWidth;return window.undefined}function WinHeight(){if (window.innerHeight!=window.undefined) return window.innerHeight;if (document.compatMode=='CSS1Compat') return document.documentElement.clientHeight;if (document.body) return document.body.clientHeight;return window.undefined}function FitPic(){iWidth=WinWidth();iHeight=WinHeight();iWidth = document.images[0].width - iWidth;iHeight = document.images[0].height - iHeight;window.resizeBy((iWidth),(iHeight))self.focus()} </script></head><body onload="FitPic()"><a href="javascript:self.close()"title="<?php _e('Click to close',$lg_text_domain);?>">-<img src="<?php echo str_replace("","%20",$lg_gallery->address.$_GET['folder'].$_GET['image']);?>"alt="<?php echo $_GET['image'];?>"/>+<img src="<?php echo str_replace("","%20",$lg_gallery->address.esc_attr($_GET['folder']).esc_attr($_GET['image']));?>"alt="<?php echo $_GET['image'];?>"/></a></body></html><?php?> | |
