| DetailsAffected Software:Subscribe to Comments Plugin Fixed in Version:2.1 Issue Type:Cross Site Scripting (XSS) Original Code:Found Here DescriptionA familiar symptom here with the same ole result. In the vulnerable code we see a call to $_SERVER['REQUEST_URI']. For some reason,many developers assume REQUEST_URI cannot be tainted and used for XSS. REQUEST_URI will not only include the path to current php file,it will also include any querystring parameters in the URI as well. Here’s a few examples of what REQUEST_URI will return: http://spotthevuln.com/blah/
results in –>/ http://spotthevuln.com/blah//index.php results in –>/blah/index.php http://spotthevuln.com/blah/index.php?qs=value results in –>/blah/index.php?qs=value http://spotthevuln.com/blah/index.php/qs1/qs2 results in –>/blah/index.php/qs1/qs2
As you can see an attacker can easily taint the REQUEST_URI value,using it in XSS attacks. The developers addressed this vulnerability by encoding calls to REQUEST_URI. Developers Solution<?phpfunction show_subscription_checkbox ($id='0'){global $sg_subscribe;sg_subscribe_start();if ( $sg_subscribe->checkbox_shown ) return $id;if ( !$email = $sg_subscribe->current_viewer_subscription_status() ):?><?php ?><?php ?><?php ?><p <?php if ($sg_subscribe->clear_both) echo 'style="clear:both;"';?>class="subscribe-to-comments"> <input type="checkbox"name="subscribe"id="subscribe"value="subscribe"style="width:auto;"<?php if ($sg_subscribe->default_subscribed) echo 'checked="checked"';?>/> <label for="subscribe"><?php echo $sg_subscribe->not_subscribed_text;?></label></p><?php ?><?php elseif ( $email == 'admin' &¤t_user_can('manage_options') ):?><?php ?><?php ?><?php ?><p <?php if ($sg_subscribe->clear_both) echo 'style="clear:both;"';?>class="subscribe-to-comments"><?php echo str_replace('[manager_link]',$sg_subscribe->manage_link($email,true,false),$sg_subscribe->author_text);?></p><?php else:?><?php ?><?php ?><?php ?><p <?php if ($sg_subscribe->clear_both) echo 'style="clear:both;"';?>class="subscribe-to-comments"><?php echo str_replace('[manager_link]',$sg_subscribe->manage_link($email,true,false),$sg_subscribe->subscribed_text);?></p><?php ?><?php endif;$sg_subscribe->checkbox_shown = true;return $id}function show_manual_subscription_form (){global $id,$sg_subscribe,$user_email;sg_subscribe_start();$sg_subscribe->show_errors('solo_subscribe','<div class="solo-subscribe-errors">','</div>',__('<strong>Error:</strong>','subscribe-to-comments'),'<br />');if ( !$sg_subscribe->current_viewer_subscription_status() ):get_currentuserinfo();?><?php ?><?php ?><?php ?>-<form action="http://<?php echo $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI'] ?>"method="post">+<form action="http://<?php echo $_SERVER['HTTP_HOST'] . wp_specialchars($_SERVER['REQUEST_URI']);?>"method="post"><input type="hidden"name="solo-comment-subscribe"value="solo-comment-subscribe"/><input type="hidden"name="postid"value="<?php echo $id;?>"/>-<input type="hidden"name="ref"value="<?php echo 'http://' . $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI'];?>"/>+<input type="hidden"name="ref"value="<?php echo urlencode('http://' . $_SERVER['HTTP_HOST'] . wp_specialchars($_SERVER['REQUEST_URI']));?>"/><p class="solo-subscribe-to-comments"><?php _e('Subscribe without commenting','subscribe-to-comments');?><br /><label for="solo-subscribe-email"><?php _e('E-Mail:','subscribe-to-comments');?><input type="text"name="email"id="solo-subscribe-email"size="22"value="<?php echo $user_email;?>"/></label><input type="submit"name="submit"value="<?php _e('Subscribe','subscribe-to-comments');?>"/></p></form><?php ?>DetailsAffected Software:WordPress (Core) Fixed in Version:2.3 Issue Type:Cross Site Scripting (XSS) Original Code:Found Here DescriptionInteresting bug here,not because of the technical details associated with the bug but more of the process associated with code check in process. First,the bug is simple,after looking through the code we see a single line that outputs POST and GET parameters into HTML markup. Although the user controlled values are enclosed within HTML comments,the comment tokens do little to prevent XSS as an attacker can simply break out of the HTML comments. What’s surprising here is the line of code that causes the XSS appears to be some kind of debugging statement. Debugging is a necessary function in any large/complex software project. Developers should have a standard way to tag debugging functionality so that it doesn’t accidently make its way into production builds. If the debugging functionality were tagged/marked,it would be easy to identify and could have been flagged by simple automation before/during check in time. This bug also shows that the organization likely doesn’t have robust testing program. A quick buddy test of this page would have likely identified this issue and it could have been removed. I’m also guessing that this debugging functionality doesn’t have any test cases associated with it,which would have raised more red flags. Developers Solution<?phpfunction convert_all_confirm(){print '<div class="narrow">';print '<h3>' . __('Confirm') . '</h3>';print '<p>' . __('You are about to convert all categories to tags. Are you sure you want to continue?') . '</p>';print '<form action="admin.php?import=wp-cat2tag"method="post">';print '<p style="text-align:center"class="submit"><input type="submit"value="' . __('Yes') . '"name="yes_convert_all_cats"/> <input type="submit"value="' . __('No') . '"name="no_dont_do_it"/></p>';print '</form>';print '</div>'}function convert_all(){global $wpdb;$wpdb->query("UPDATE $wpdb->term_taxonomy SET taxonomy = 'post_tag',parent = 0 WHERE taxonomy = 'category'");clean_category_cache($category->term_id)}function init(){-echo '<!--';print_r($_POST);print_r($_GET);echo '-->';if (isset($_POST['maybe_convert_all_cats'])){$step = 3} elseif (isset($_POST['yes_convert_all_cats'])){$step = 4} elseif (isset($_POST['no_dont_do_it'])){die('no_dont_do_it')} else{$step = (isset($_GET['step'])) ? (int) $_GET['step']:1}$this->header();if (!current_user_can('manage_categories')){print '<div class="narrow">';print '<p>' . __('Cheatin’uh?') . '</p>';print '</div>'} else{switch ($step){case 1:$this->welcome();break;case 2:$this->convert_them();break;case 3:$this->convert_all_confirm();break;case 4:$this->convert_all();break}}$this->footer()}function WP_Categories_to_Tags(){// Do nothing.}}$wp_cat2tag_importer = new WP_Categories_to_Tags();register_importer('wp-cat2tag',__('Categories to Tags Converter'),__('Convert existing categories to tags,selectively.'),array(&$wp_cat2tag_importer,'init'));?>DetailsAffected Software:WordPress Fixed in Version:2.1 Issue Type:HTTP Header Injection Original Code:Found Here DescriptionThis week’s bug was a Set-Cookie/HTTP Header injection bug in WordPress Core. Looking at the code,we see that the WordPress Developers assign several variables directly from the $_POST body. These assignments are listed below: $comment_author = trim($_POST['author']); $comment_author_email = trim($_POST['email']); $comment_author_url = trim($_POST['url']); $comment_content = trim($_POST['comment']);
Each of these variables should now be considered tainted and should be sanitized/encoded before using their values in any operations. It seems that the WordPress developers were aware that the values assigned to the variables listed above could not be trusted and proceeded to escape the values before using them in database related operations. Some examples of the sanitization can are provided below: $comment_author = $wpdb->escape($user_identity); $comment_author_email = $wpdb->escape($user_email); $comment_author_url = $wpdb->escape($user_url);
Unfortunately,the developers utilized the incorrect sanitizing routines for the tainted values in the setcookie API. The developers used stripslashes() to sanitize the user controlled value before passing it to the setcookie() API. Although stripslashes can help prevent other types of vulnerabilities,it was not intended to defend against HTTP HEADER injection or cookie poisoning. By injecting a series of CRLF (%0d%0a) characters,an attacker could use this bug to inject arbitrary headers into the HTTP response and in some cases use this header injection bug for XSS. The WordPress developers addressed this issue by passing the attacker controlled value to the clean_url() function before allowing it in the setcookie. The full source for clean_url() can be found here,however the more interesting sanitizing routines are provided below: $url = preg_replace(‘|[^a-z0-9-~+_.?#=!&;,/:%@$\|*\'()\\x80-\\xff]|i’,”,$url); $strip = array(‘%0d’,‘%0a’,‘%0D’,‘%0A’); $url = _deep_replace($strip,$url); $url = str_replace(‘;//’,‘://’,$url);
Developers Solution<?phprequire( dirname(__FILE__) . '/wp-config.php' );nocache_headers();$comment_post_ID = (int) $_POST['comment_post_ID'];$status = $wpdb->get_row("SELECT post_status,comment_status FROM $wpdb->posts WHERE ID = '$comment_post_ID'");if ( empty($status->comment_status) ){do_action('comment_id_not_found',$comment_post_ID);exit} elseif ( 'closed' == $status->comment_status ){do_action('comment_closed',$comment_post_ID);die( __('Sorry,comments are closed for this item.') )} elseif ( 'draft' == $status->post_status ){do_action('comment_on_draft',$comment_post_ID);exit}$comment_author = trim($_POST['author']);$comment_author_email = trim($_POST['email']);$comment_author_url = trim($_POST['url']);$comment_content = trim($_POST['comment']);// If the user is logged inget_currentuserinfo();if ( $user_ID ):$comment_author = $wpdb->escape($user_identity);$comment_author_email = $wpdb->escape($user_email);$comment_author_url = $wpdb->escape($user_url);else:if ( get_option('comment_registration') )die( __('Sorry,you must be logged in to post a comment.') );endif;$comment_type = '';if ( get_settings('require_name_email') &&!$user_ID ){if ( 6 >strlen($comment_author_email) || '' == $comment_author )die( __('Error:please fill the required fields (name,email).') );elseif ( !is_email($comment_author_email))die( __('Error:please enter a valid email address.') )}if ( '' == $comment_content )die( __('Error:please type a comment.') );$commentdata = compact('comment_post_ID','comment_author','comment_author_email','comment_author_url','comment_content','comment_type','user_ID');wp_new_comment( $commentdata );if ( !$user_ID ):setcookie('comment_author_' . COOKIEHASH,stripslashes($comment_author),time() + 30000000,COOKIEPATH,COOKIE_DOMAIN);setcookie('comment_author_email_' . COOKIEHASH,stripslashes($comment_author_email),time() + 30000000,COOKIEPATH,COOKIE_DOMAIN);-setcookie('comment_author_url_' . COOKIEHASH,stripslashes($comment_author_url),time() + 30000000,COOKIEPATH,COOKIE_DOMAIN);+setcookie('comment_author_url_' . COOKIEHASH,stripslashes(clean_url($comment_author_url)),time() + 30000000,COOKIEPATH,COOKIE_DOMAIN);endif;$location = ( empty( $_POST['redirect_to'] ) ) ? get_permalink( $comment_post_ID ):$_POST['redirect_to'];wp_redirect( $location );?>
DetailsAffected Software:OpenFire Fixed in Version:3.7.0b Issue Type:XSS Original Code: Found Here DescriptionThis week’s bug was an XSS vulnerability caused by the improper escaping of an HTML attribute. It’s obvious that the developers attempted to protect their software from XSS vulnerabilities. They even wrote their own XSS sanitizing method (escapeHTMLTags). The escapeHTMLTags() method is simple,strip out <and >characters and return the string. Unfortunately,this simple pattern isn’t sufficient in defending against all XSS vulnerabilities. There is a bit of tracing that is required to understand this bug,so let’s start the tracing. The bug begins with the following variable assignment: String username = ParamUtils.getParameter(request,“username”);
The username value is assigned directly from the HTTP request. Later,the username variable is escaped using the custom escapeHTMLTags() function. The escaping occurs in the following line: username = org.jivesoftware.util.StringUtils.escapeHTMLTags(username);
Later in the code,the escaped username value is used in the markup as part of an HTML attribute. The vulnerable line is presented below: <input size=”15″maxlength=”50″value=”<%= (username != null ? username:“”) %>”>
The line above checks to see if the username variable has been assigned a value. If the username variable contains a value,it is displayed in the markup as the value attribute for an input field. While sanitizing the <and >characters would prevent an attacker closing the input field and starting a new html tag,it doesn’t prevent an attacker from closing off the attribute value and injecting a new HTML attribute for the input field. Some consider injection into a input field to be unexploitable (or limited to certain browsers),check out Gareth Heyes blog post about exploiting text fields with new HTML5 events http://www.thespanner.co.uk/2009/12/06/html5-new-xss-vectors/ Developers Solution public static String escapeHTMLTags(String in){ if (in == null){ return null; } char ch; int i = 0; int last = 0; char[] input = in.toCharArray(); int len = input.length; StringBuilder out = new StringBuilder((int)(len * 1.3)); for (;i <len;i++){ ch = input[i]; if (ch >'>'){ } else if (ch == '<'){if (i >last){out.append(input,last,i - last);} last = i + 1;out.append(LT_ENCODE); } else if (ch == '>'){if (i >last){out.append(input,last,i - last);} last = i + 1;out.append(GT_ENCODE); } else if (ch == '\n'){if (i >last){out.append(input,last,i - last);} last = i + 1;out.append("<br>"); } } if (last == 0){ return in; } if (i >last){ out.append(input,last,i - last); } return out.toString(); }... <snip>...<% // get parameters String username = ParamUtils.getParameter(request,"username"); String password = ParamUtils.getParameter(request,"password"); String url = ParamUtils.getParameter(request,"url"); url = org.jivesoftware.util.StringUtils.escapeHTMLTags(url); // SSO between cluster nodes String secret = ParamUtils.getParameter(request,"secret"); String nodeID = ParamUtils.getParameter(request,"nodeID"); String nonce = ParamUtils.getParameter(request,"nonce"); // The user auth token: AuthToken authToken = null; // Check the request/response for a login token Map<String,String>errors = new HashMap<String,String>(); if (ParamUtils.getBooleanParameter(request,"login")){ String loginUsername = username; if (loginUsername != null){ loginUsername = JID.escapeNode(loginUsername); } try{ if (secret != null &&nodeID != null){if (StringUtils.hash(AdminConsolePlugin.secret).equals(secret) &&ClusterManager.isClusterMember(Base64.decode(nodeID,Base64.URL_SAFE))){authToken = new AuthToken(loginUsername);} else if ("clearspace".equals(nodeID) &&ClearspaceManager.isEnabled()){ClearspaceManager csmanager = ClearspaceManager.getInstance();String sharedSecret = csmanager.getSharedSecret();if (nonce == null || sharedSecret == null || !csmanager.isValidNonce(nonce) || !StringUtils.hash(loginUsername + ":"+ sharedSecret + ":"+ nonce).equals(secret)){throw new UnauthorizedException("SSO failed. Invalid secret was provided");} authToken = new AuthToken(loginUsername);} else{throw new UnauthorizedException("SSO failed. Invalid secret or node ID was provided");} } else{// Check that a username was provided before trying to verify credentials if (loginUsername != null){if (LoginLimitManager.getInstance().hasHitConnectionLimit(loginUsername,request.getRemoteAddr())){throw new UnauthorizedException("User '"+ loginUsername +"' or address '"+ request.getRemoteAddr() + "' has his login attempt limit.");} if (!AdminManager.getInstance().isUserAdmin(loginUsername,true)){throw new UnauthorizedException("User '"+ loginUsername + "' not allowed to login.");} authToken = AuthFactory.authenticate(loginUsername,password);} else{errors.put("unauthorized",LocaleUtils.getLocalizedString("login.failed.unauthorized"));} }... <snip>... // Escape HTML tags in username to prevent cross-site scripting attacks. This // is necessary because we display the username in the page below. username = org.jivesoftware.util.StringUtils.escapeHTMLTags(username);%><!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"><html><head><title><%= AdminConsole.getAppName() %><fmt:message key="login.title"/></title><script language="JavaScript"type="text/javascript"><!--// break out of framesif (self.parent.frames.length != 0){self.parent.location=document.location} function updateFields(el){ if (el.checked){document.loginForm.username.disabled = true;document.loginForm.password.disabled = true; } else{document.loginForm.username.disabled = false;document.loginForm.password.disabled = false;document.loginForm.username.focus(); } }//--></script> <link rel="stylesheet"href="style/global.css"type="text/css"> <link rel="stylesheet"href="style/login.css"type="text/css"></head><body><form action="login.jsp"name="loginForm"method="post"><% if (url != null){try{%> <input type="hidden"name="url"value="<%= url %>"><% } catch (Exception e){Log.error(e);} } %><input type="hidden"name="login"value="true"><div align="center"> <!-- BEGIN login box --> <div id="jive-loginBox"> <div align="center"id="jive-loginTable"> <span id="jive-login-header"style="background:transparent url(images/login_logo.gif) no-repeat left;padding:29px 0 10px 205px;"> <fmt:message key="admin.console"/> </span> <div style="text-align:center;width:380px;"> <table cellpadding="0"cellspacing="0"border="0"align="center"><tr><td align="right"class="loginFormTable"><table cellpadding="2"cellspacing="0"border="0"><noscript> <tr> <td colspan="3"> <table cellpadding="0"cellspacing="0"border="0"> <tr valign="top"> <td><img src="images/error-16x16.gif"width="16"height="16"border="0"alt=""vspace="2"></td> <td><div class="jive-error-text"style="padding-left:5px;color:#cc0000;"><fmt:message key="login.error"/></div></td> </tr> </table> </td> </tr></noscript><% if (errors.size() >0){%> <tr> <td colspan="3"> <table cellpadding="0"cellspacing="0"border="0"> <% for (String error:errors.values()){%> <tr valign="top"> <td><img src="images/error-16x16.gif"width="16"height="16"border="0"alt=""vspace="2"></td> <td><div class="jive-error-text"style="padding-left:5px;color:#cc0000;"><%= error%></div></td> </tr> <% } %> </table> </td> </tr><% } %><tr>+ <td><input type="text"name="username"size="15"maxlength="50"id="u01"value="<%= (username != null ? StringUtils.removeXSSCharacters(username):"") %>"></td>- <td><input type="text"name="username"size="15"maxlength="50"id="u01"value="<%= (username != null ? username:"") %>"></td> <td><input type="password"name="password"size="15"maxlength="50"id="p01"></td> <td align="center"><input type="submit"value=" <fmt:message key="login.login"/> "></td></tr><tr valign="top"> <td class="jive-login-label"><label for="u01"><fmt:message key="login.username"/></label></td> <td class="jive-login-label"><label for="p01"><fmt:message key="login.password"/></label></td> <td> </td></tr></table></td></tr><tr><td align="right"><div align="right"id="jive-loginVersion"><%= AdminConsole.getAppName() %>,<fmt:message key="login.version"/>:<%= AdminConsole.getVersionString() %></div></td></tr> </table> </div> </div> </div> <!-- END login box --></div></form><script language="JavaScript"type="text/javascript"><!-- if (document.loginForm.username.value == ''){ document.loginForm.username.focus(); } else{ document.loginForm.password.focus(); }//--></script></body></html>DetailsAffected Software:Dojox Fixed in Version:1.4.1 Issue Type:XSS Original Code: Found Here DescriptionThe code sample presented this week have a few XSS vulnerabilities. The first XSS bug is caused when $_GET[‘messId’] doesn’t match any of the switch cases. The default behavior is to throw a friendly error message indicating the messId is unknown along with the value passed via the querystring. Unfortunately,the error message can also contain attacker controlled HTML/SCRIPT resulting in XSS. The two other XSS bugs fixed by the developers in this patch are classic XSS. Both issues take user/attacker controlled variables and display the variables in markup without sanitization/encoding. Although the XSS bugs are fairly straight forward,what I find interesting in this example is why this page is here in the first place. The code don’t seem to provide any useful functionality (other than to provide a place for attackers to abuse XSS). If you were a security engineer assigned to audit this page,it might be a good idea to ask WHY this page exists in the first place. It turns out that this page is indeed a test/debugging page that is included with dojox,offering no functionality intended for production users. Think long and hard before exposing test and debug functionality in your production environment. Check production systems for example/testing/debugging functionality and disable/remove this functionality if possible. Code developed for testing and debugging rarely undergoes the scrutiny of production code and will like contain security issues. 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
| <?php// this just bounces a message as a response,and optionally emulates network latency. // default delay is 0 sec,to change: // getResponse.php?delay= [Int milliseconds ]// to change the message returned // getResponse.php?mess=whatever%20string%20you%20want. // to select a predefined message // getResponse.php?messId= 0error_reporting (E_ALL ^ E_NOTICE ); $delay = 1;// 1 micro second to avoid zero division in messId 2if (isset ($_GET ['delay' ]) &&is_numeric ($_GET ['delay' ])){$delay = (intval ($_GET ['delay' ]) * 1000); }if (isset ($_GET ['messId' ]) &&is_numeric ($_GET ['messId' ])){switch ($_GET ['messId' ]){case 0: echo "<h3>WARNING This should NEVER be seen,delayed by 2 sec!</h3>"; $delay = 2; break; case 1: echo "<div dojotype='dijit.TestWidget'>Testing attr ('href',... )</div>"; break; case 2: echo "<div dojotype='dijit.TestWidget'>Delayed attr ('href',... ) test</div> <div dojotype='dijit.TestWidget'>Delayed by ". ($delay/1000000) . "sec.</div>";break; case 3: echo "IT WAS the best of times,it was the worst of times,it was the age of wisdom,it was the age of foolishness,it was the epoch of belief,it was the epoch of incredulity,it was the season of Light,it was the season of Darkness,it was the spring of hope,it was the winter of despair,we had everything before us,we had nothing before us,we were all going direct to Heaven,we were all going direct the other way -- in short,the period was so far like the present period,that some of its noisiest authorities insisted on its being received,for good or for evil,in the superlative degree of comparison only"; break; case 4: echo "There were a king with a large jaw and a queen with a plain face,on the throne of England;there were a king with a large jaw and a queen with a fair face,on the throne of France. In both countries it was clearer than crystal to the lords of the State preserves of loaves and fishes,that things in general were settled for ever."; break; case 5: echo "It was the year of Our Lord one thousand seven hundred and seventy- five. Spiritual revelations were conceded to England at that favoured period,as at this. Mrs. Southcott had recently attained her five-and- twentieth blessed birthday,of whom a prophetic private in the Life Guards had heralded the sublime appearance by announcing that arrangements were made for the swallowing up of London and Westminster. Even the Cock-lane ghost had been laid only a round dozen of years,after rapping out its messages,as the spirits of this very year last past (supernaturally deficient in originality ) rapped out theirs. Mere messages in the earthly order of events had lately come to the English Crown and People,from a congress of British subjects in America:"; break; default: -echo "unknown messId:{$_GET['messId']}";+echo "unknown messId:". htmlentities($_GET['messId']);}}if (isset ($_GET ['bounceGetStr' ]) && $_GET ['bounceGetStr' ]){-echo "<div id='bouncedGetStr'>{$_SERVER["QUERY_STRING"]}</div>";+echo "<div id='bouncedGetStr'>".htmlentities($_SERVER["QUERY_STRING"])."</div>";}if (isset ($_GET ['message' ]) &&$_GET ['message' ]){-echo $_GET['message'];+echo htmlentities($_GET['message']);}usleep ($delay ); ?> |
DetailsAffected Software:WordPress (core) Fixed in Version:1.2 Issue Type:XSS Original Code: Found Here DescriptionLooking at this code made me smile,its about 6 years old. There’s a lot going on here and quite a few issues. The first thing that jumped out at me was the use of $HTTP_POST_FILES. $HTTP_POST_FILES means were working with user controlled files. There are tons of things that can go wrong when dealing with user/attacker controlled files (a list too long to go into here). Lets hope the WordPress devs are on their A-game here. Looking at the patch submitted by the WordPress developers,we see that they changed references to $HTTP_POST_FILES to the superglobal $_FILES. Within the $_FILES array there are a couple indexes that are commonly used. These indexes are: [name]
[type]
[tmp_name]
[error]
[size]
Name,type,and size are all controlled by the user/attacker,so the WordPress developers should be wary when dealing with these values. Surprisingly (or unsurprisingly,depending on your point of view),this patch doesn’t contain seem to contain any robust validation of data associated with the uploaded file data. Instead,the defenses put in place here seem to be centered around replacing a poor validation/sanitization routine with a more robust encoding routine which prevents a XSS vulnerability. The replaced sanitization routine and the XSS bug are presented in the following lines: -$imgdesc = str_replace(‘”‘,‘&quot;’,$_POST['imgdesc']);
+$imgdesc = htmlentities2($imgdesc);
class=”uploadform”value=”<?php echo $imgdesc;?>”/>
What’s surprising is although the WordPress developers prevented this single XSS vulnerability,there is a large number of XSS vulnerabilities in this file. The most obvious symptom is “<?php echo $_REQUEST[] ?>. Additionally,the loose validation of the user uploaded file is concerning,especially with the number of problems that can be encountered when dealing with user controlled files. Maybe the varsity team was on vacation when this patch went in. 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
| <?php //Makes sure they choose a file
//print_r($HTTP_POST_FILES);
//die();
- $imgalt = (isset($_POST['imgalt'])) ? $_POST['imgalt']:$imgalt;
-
- $img1_name = (strlen($imgalt)) ? $_POST['imgalt']:$HTTP_POST_FILES['img1']['name'];
- $img1_type = (strlen($imgalt)) ? $_POST['img1_type']:$HTTP_POST_FILES['img1']['type'];
- $imgdesc = str_replace('"','&quot;',$_POST['imgdesc']);
+$imgalt = basename( (isset($_POST['imgalt'])) ? $_POST['imgalt']:'' );
+
+$img1_name = (strlen($imgalt)) ? $imgalt:basename( $_FILES['img1']['name'] );
+$img1_type = (strlen($imgalt)) ? $_POST['img1_type']:$_FILES['img1']['type'];
+$imgdesc = htmlentities2($imgdesc);
$imgtype = explode(".",$img1_name);
$imgtype = strtolower($imgtype[count($imgtype)-1]);
if (in_array($imgtype,$allowed_types) == false) {
die(sprintf(__('File %1$s of type %2$s is not allowed.') ,$img1_name,$imgtype));
}
if (strlen($imgalt)) {
$pathtofile = get_settings('fileupload_realpath')."/".$imgalt;
-$img1 = $_POST['img1'];
+$img1 = $_POST['img1']['tmp_name'];
} else {
$pathtofile = get_settings('fileupload_realpath')."/".$img1_name;
-$img1 = $HTTP_POST_FILES['img1']['tmp_name'];
+$img1 = $_FILES['img1']['tmp_name'];
}
// makes sure not to upload duplicates,rename duplicates
$i = 1;
$pathtofile2 = $pathtofile;
$tmppathtofile = $pathtofile2;
$img2_name = $img1_name;
while (file_exists($pathtofile2)) {
$pos = strpos($tmppathtofile,'.'.trim($imgtype));
$pathtofile_start = substr($tmppathtofile,0,$pos);
$pathtofile2 = $pathtofile_start.'_'.zeroise($i++,2).'.'.trim($imgtype);
$img2_name = explode('/',$pathtofile2);
$img2_name = $img2_name[count($img2_name)-1];
}
if (file_exists($pathtofile) &&!strlen($imgalt)) {
$i = explode(' ',get_settings('fileupload_allowedtypes'));
$i = implode(',',array_slice($i,1,count($i)-2));
$moved = move_uploaded_file($img1,$pathtofile2);
// if move_uploaded_file() fails,try copy()
if (!$moved) {
$moved = copy($img1,$pathtofile2);
}
if (!$moved) {
die(sprintf(__("Couldn't upload your file to %s."),$pathtofile2));
} else {
chmod($pathtofile2,0666);
@unlink($img1);
}
//
// duplicate-renaming function contributed by Gary Lawrence Murphy
?>
<p><strong><?php __('Duplicate File?') ?></strong></p>
<p><b><em><?php printf(__("The filename '%s' already exists!"),$img1_name);?></em></b></p>
<p><?php printf(__("Filename '%1\$s' moved to '%2\$s'"),$img1,"$pathtofile2 - $img2_name") ?></p>
<p><?php _e('Confirm or rename:') ?></p>
<form action="upload.php"method="post"enctype="multipart/form-data">
<input type="hidden"name="MAX_FILE_SIZE"value="<?php echo get_settings('fileupload_maxk') *1024 ?>"/>
<input type="hidden"name="img1_type"value="<?php echo $img1_type;?>"/>
<input type="hidden"name="img1_name"value="<?php echo $img2_name;?>"/>
<input type="hidden"name="img1_size"value="<?php echo $img1_size;?>"/>
<input type="hidden"name="img1"value="<?php echo $pathtofile2;?>"/>
<input type="hidden"name="thumbsize"value="<?php echo $_REQUEST['thumbsize'];?>"/>
<input type="hidden"name="imgthumbsizecustom"value="<?php echo $_REQUEST['imgthumbsizecustom'];?>"/>
<?php _e('Alternate name:') ?><br /><input type="text"name="imgalt"size="30"value="<?php echo $img2_name;?>"/><br />
<br />
<?php _e('Description:') ?><br /><input type="text"name="imgdesc"size="30"value="<?php echo $imgdesc;?>"/>
<br />
<input type="submit"name="submit"value="<?php _e('Rename') ?>"/>
</form>
</div> |
DetailsAffected Software:Members-only WordPress plugin Fixed in Version:0.6.7 Issue Type:SQL Injection Original Code: Found Here DescriptionThis week’s vuln was an easy one  The vulnerability affected the “members-only” plug-in for WordPress and was patched in version 0.6.7. There are a couple changes in the diff,but the most important change (from a security standpoint) is the transition from a string-built,dynamic SQL statement to a prepared statement. The following lines show the SQL injection bug: $feedkey = $_GET['feedkey'];
… <snip>…
$find_feedkey = $wpdb->get_results(“SELECT umeta_id FROM $wpdb->usermeta WHERE meta_value = ‘$feedkey’”);
The members-only developers assigned a value for the $feedkey variable directly from the querystring. They then used the attacker controlled value to build a SQL statement,using the $feedkey value to complete the SQL WHERE clause. The symptoms presented above are classic SQL injection. I’m happy to see that the members-only developers chose to use prepared statements to fix the SQL injection. There are a few other fixes here,but the SQL injection was the most important security fix. 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
| -if (empty($userdata->ID) &&$members_only_opt['feed_access'] != 'feednone') //Check if user is logged in or Feed Keys is required
+if (empty($userdata->ID) || $members_only_opt['feed_access'] != 'feednone') //Check if user is logged in or Feed Keys is required
{
$feedkey = $_GET['feedkey'];
if (!empty($feedkey))
{
// Check if Feed Key is in the Database
-$find_feedkey = $wpdb->get_results("SELECT umeta_id FROM $wpdb->usermeta WHERE meta_value = '$feedkey'");
+$find_feedkey = $wpdb->get_results( $wpdb->prepare(
+ "SELECT umeta_id FROM $wpdb->usermeta WHERE meta_value = %s",
+ $feedkey
+) );
if (!empty($find_feedkey) &&$members_only_opt['feed_access'] == 'feedkeys') //If Feed Key is found and using Feed Keys
{
$feedkey_valid = TRUE;
if (empty($feedkey) &&$members_only_opt['feed_access'] == 'feedkeys')
{
$feed = members_only_create_feed('No Feed Key Found',$errormsg['feedkey_missing']);
-header("Content-Type:application/xml;charset=ISO-8859-1");
+header("Content-Type:application/xml;charset=". get_bloginfo( 'charset' ),true);
echo $feed;
exit;
}
elseif ($feedkey_valid == FALSE &&$members_only_opt['feed_access'] == 'feedkeys')
{
$feed = members_only_create_feed('Feed Key is Invalid',$errormsg['feedkey_invalid']);
-header("Content-Type:application/xml;charset=ISO-8859-1");
+header("Content-Type:application/xml;charset=". get_bloginfo( 'charset' ),true);
echo $feed;
exit;
}
if (empty($feedkey) &&$members_only_opt['feed_access'] == 'feedkeys')
{
$feed = members_only_create_feed('No Feed Key Found',$errormsg['feedkey_missing']);
-header("Content-Type:application/xml;charset=ISO-8859-1");
+header("Content-Type:application/xml;charset=". get_bloginfo( 'charset' ),true);
echo $feed;
exit;
} |
DetailsAffected Software:WordPress-to-lead for Salesforce CRM Fixed in Version:1.0.2 Issue Type:Cross Site Scripting (XSS) Original Code: Found Here DescriptionThis week’s vulnerability affected the WordPress-to-lead for Salesforce plugin. Looking at the vulnerable code,we see that stripslashes was applied before echoing attacker controlled content into the HTML markup. Stripslashes was applied in the following lines: $val = strip_tags(stripslashes($_POST[$id])); $content .= “\t”.’<label for=”sf_’.$id.’”>’.esc_html(stripslashes($input['label'])).’:'; $content .= “\t”.’<input type=”submit”value=”‘.esc_attr($submit).’”/>’.”\n”;
Unfortunately,stripslashes doesn’t help protect against XSS. It’s uncertain whether the original developer thought stripslashes would help protect against XSS. Regardless of the developer’s intentions,the example does bring a good principle to light. Developers (and security professionals) should understand what security based APIs provide. It can be difficult to understand which API is most appropriate for various situations. Having guidance for non security focused developers is important. As security professionals,the benefits of an API usage can be obvious. For a non security focused developer,the number of security APIs can be daunting,much less understanding when to call one API versus another. This vulnerability was addressed by using the proper escaping APIs as shown in the patch below. 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
| } else if ($is_sidebar &&$options ['usecss' ]) {$content .= '<style type="text/css"> .sidebar form.w2llead {clear:none;text-align:left; }.sidebar .w2linput,#sidebar .w2llabel {float:none;display:inline; }.sidebar .w2llabel.error {color:#f00; }.sidebar .w2llabel {margin:4px 0; }.sidebar .w2linput.text {width: 95%;height:18px;margin:4px 0; }.sidebar .w2linput.textarea {width: 95%;height:50px;margin:10px 0; }.sidebar .w2linput.submit {margin:10px 0 0 0; }#salesforce {margin:3px 0 0 0;color:#aaa; }#salesforce a {color:# 999; }</style>';}$sidebar = ''; if ($is_sidebar )$sidebar = ' sidebar'; $content .= "\n".'<form method="post">'."\n"; foreach ($options ['inputs' ] as $id =>$input ) {if (!$input ['show' ])continue; $val = ''; if (isset ($_POST [$id ]))-$val = strip_tags(stripslashes($_POST[$id]));+$val = esc_attr(strip_tags(stripslashes($_POST[$id])));$error = ' '; if ($input ['error' ])$error = ' error '; -$content .= "\t".'<label for="sf_'.$id.'">'.stripslashes($input['label']).':';+$content .= "\t".'<label for="sf_'.$id.'">'.esc_html(stripslashes($input['label'])).':';if ($input ['required' ])$content .= ' *'; $content .= '</label>'."\n"; if ($input ['type' ] == 'text' ) {$content .= "\t".'<input value="'.$val.'"id="sf_'.$id.'"name="'.$id.'"type="text"/><br/>'."\n\n"; } else if ($input ['type' ] == 'textarea' ) {$content .= "\t".'<br/>'."\n\t".'<textarea id="sf_'.$id.'"name="'.$id.'">'.$val.'</textarea><br/>'."\n\n"; }}$submit = stripslashes ($options ['submitbutton' ]); if (empty ($submit ))$submit = "Submit"; -$content .= "\t".'<input type="submit"name="w2lsubmit"value="'.$submit.'"/>'."\n";+$content .= "\t".'<input type="submit"name="w2lsubmit"value="'.esc_attr($submit).'"/>'."\n";$content .= '</form>'."\n"; $reqtext = stripslashes ($options ['requiredfieldstext' ]); if (!empty ($reqtext ))$content .= '<p id="requiredfieldsmsg"><sup>*</sup>'.$reqtext.'</p>'; $content .= '<div id="salesforce"><small>Powered by <a href="http://www.salesforce.com/">Salesforce CRM</a></small></div>'; return $content; }function submit_salesforce_form ($post,$options ) {global $wp_version; if (!isset ($options ['org_id' ]) || empty ($options ['org_id' ]))return false; $post ['oid' ] = $options ['org_id' ]; $post ['lead_source' ] = $options ['source' ]; $post ['debug' ] = 0; // Set SSL verify to false because of server issues. $args = array ('body' =>$post, 'headers' =>array ('user-agent' =>'WordPress-to-Lead for Salesforce plugin - WordPress/'.$wp_version.';'.get_bloginfo ('url' ), ), 'sslverify' =>false, ); $result = wp_remote_post ('https://www.salesforce.com/servlet/servlet.WebToLead?encoding=UTF- 8',$args ); if ($result ['headers' ]['is-processed' ] == "true" )return true; else return false; } |
DetailsAffected Software:PunBB Fixed in Version:1.2 Issue Type:Cross Site Scripting (XSS) Original Code: Found Here DescriptionThis weeks’example was a XSS bug that affected PunBB. The vulnerable code took attacker controlled variables directly from POST parameters and used those values for various operations. Specifically,the $_POST[‘prune_sticky’] value was used in several places without any form of sanitization. Looking through the patch submitted by the PunBB developers we see that the unsantized value was passed to a function named prune() and also echoed in HTML markup. PHP echo of a $_POST variable is a classic symptom of XSS. The PunBB developers addressed this issue by sanitizing the $_POST['prune_sticky'] value before echoing its value in HTML markup. The developer forces the $_POST[‘prune_sticky’] value to either 1 or 0,which eliminates the possibility or arbitrary script being injected via the $prune_sticky variable. The $_POST[‘prune_sticky’] value is sanitized here: $prune_sticky = isset($_POST['prune_sticky']) ? ’1′:’0′;
In addition to the changes made to PHP echo there were other changes checked in the by the PunBB developers,most notably the sanitizing of values passed to the prune() function. Using the code snippet provided here,it’s difficult to understand exactly what is accomplished by the prune() function. Further variable tracing will be needed in order to determine the danger associated with passing a tainted value to prune(). The developers however felt it was necessary to sanitize the $prune_sticky value before passing it to prune(). 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
| if (isset($_GET['action']) || isset($_POST['prune']) || isset($_POST['prune_comply']))
{
if (isset($_POST['prune_comply']))
{
confirm_referrer('admin_prune.php');
$prune_from = $_POST['prune_from'];
+$prune_sticky = isset($_POST['prune_sticky']) ? '1':'0';
$prune_days = intval($_POST['prune_days']);
$prune_date = ($prune_days) ? time() - ($prune_days*86400):-1;
@set_time_limit(0);
if ($prune_from == 'all')
{
$result = $db->query('SELECT id FROM '.$db->prefix.'forums') or error('Unable to fetch forum list',__FILE__,__LINE__,$db->error());
$num_forums = $db->num_rows($result);
for ($i = 0;$i <$num_forums;++$i)
{
$fid = $db->result($result,$i);
-prune($fid,$_POST['prune_sticky'],$prune_date);
+prune($fid,$prune_sticky,$prune_date);
update_forum($fid);
}
}
else
{
$prune_from = intval($prune_from);
-prune($prune_from,$_POST['prune_sticky'],$prune_date);
+prune($prune_from,$prune_sticky,$prune_date);
update_forum($prune_from);
}
// Locate any "orphaned redirect topics"and delete them
$result = $db->query('SELECT t1.id FROM '.$db->prefix.'topics AS t1 LEFT JOIN '.$db->prefix.'topics AS t2 ON t1.moved_to=t2.id WHERE t2.id IS NULL AND t1.moved_to IS NOT NULL') or error('Unable to fetch redirect topics',__FILE__,__LINE__,$db->error());
$num_orphans = $db->num_rows($result);
if ($num_orphans)
{
for ($i = 0;$i <$num_orphans;++$i)
$orphans[] = $db->result($result,$i);
$db->query('DELETE FROM '.$db->prefix.'topics WHERE id IN('.implode(',',$orphans).')') or error('Unable to delete redirect topics',__FILE__,__LINE__,$db->error());
}
redirect('admin_prune.php','Posts pruned. Redirecting …');
}
...<snip>...
<div>
<h2><span>Prune</span></h2>
<div>
<form method="post"action="admin_prune.php?action=foo">
<div>
<input type="hidden"name="prune_days"value="<?php echo $prune_days ?>"/>
-<input type="hidden"name="prune_sticky"value="<?php echo $_POST['prune_sticky'] ?>"/>
+<input type="hidden"name="prune_sticky"value="<?php echo $prune_sticky ?>"/>
<input type="hidden"name="prune_from"value="<?php echo $prune_from ?>"/>
<fieldset>
<legend>Confirm prune posts</legend>
<div>
<p>Are you sure that you want to prune all topics older than <?php echo $prune_days ?>days from <?php echo $forum ?>? (<?php echo $num_topics ?>topics)</p>
<p>WARNING! Pruning posts deletes them permanently.</p>
</div>
</fieldset>
</div>
<p><input type="submit"name="prune_comply"value="Prune"/><a href="javascript:history.go(-1)">Go back</a></p> |
DetailsAffected Software:WebChat Module for Jive Fixed in Version:August of 2008 Issue Type:Cross Site Scripting Original Code: Found Here DescriptionThis week’s vulnerability affected a webchat module created by Jive Software. The bug is straightforward, the JSP code takes an attacker controlled value and uses it to build dynamic HTML. Although the bug is straightforward,this week’s example was a great/simple exercise in identifying a vulnerable pattern and tracing to find other vulnerable patterns in the code. This week’s sample has three separate vulnerabilities that were all addressed via single patch. All these have similar symptoms/patterns (although the specifics are a bit different). Identifying vulnerable patterns and searching for these patterns in other places in code is an essential skill for security code auditors. Did you find all three bugs that were patched? 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
| public class FormUtils {
private FormUtils() {
}
public static String createAnswers(FormField formField,HttpServletRequest request) {
final StringBuffer builder = new StringBuffer();
if (formField.getType().equals(FormField.TYPE_TEXT_SINGLE)) {
String cookieValue = getCookieValueForField(formField.getVariable(),request);
String insertValue = "";
if(ModelUtil.hasLength(cookieValue)){
insertValue = "value=\""+cookieValue+"\"";
}
- builder.append("<input type=\"text\"name=\""+ formField.getVariable() + "\""+insertValue+"style=\"width:75%\">");
+builder.append("<input type=\"text\"name=\""+ formField.getVariable() + "\""+StringUtils.escapeHTMLTags(insertValue)+"style=\"width:75%\">");
}
else if (formField.getType().equals(FormField.TYPE_TEXT_MULTI)) {
builder.append("<textarea name=\""+ formField.getVariable() + "\"cols=\"30\"rows=\"3\">");
builder.append("</textarea>");
}
else if (formField.getType().equals(FormField.TYPE_LIST_SINGLE)) {
builder.append("<select name=\""+ formField.getVariable() + "\">");
Iterator iter = formField.getOptions();
String cookieValue = ModelUtil.emptyStringIfNull(getCookieValueForField(formField.getVariable(),request));
while (iter.hasNext()) {
FormField.Option option = (FormField.Option)iter.next();
String selected = option.getValue().equals(cookieValue) ? "selected":"";
- builder.append("<option value=\""+ option.getValue() + "\""+selected+">"+ option.getLabel() + "</option>");
+builder.append("<option value=\""+ StringUtils.escapeHTMLTags(option.getValue()) + "\""+selected+">"+ option.getLabel() + "</option>");
}
builder.append("</select>");
}
else if (formField.getType().equals(FormField.TYPE_BOOLEAN)) {
Iterator iter = formField.getOptions();
int counter = 0;
while (iter.hasNext()) {
FormField.Option option = (FormField.Option)iter.next();
String value = option.getLabel();
builder.append("<input type=\"checkbox\"value=\""+ value + "\"name=\""+ formField.getVariable() + counter + "\">");
builder.append(" ");
-builder.append(value);
+builder.append(StringUtils.escapeHTMLTags(value));
builder.append("<br/>");
counter++;
}
} |
|
