| 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: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: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++;
}
} |
DetailsAffected Software:Login LockDown for WordPress Fixed in Version: 1.5 Issue Type:SQL Injection Original Code: Found Here DescriptionThis week’s code sample comes from the “Login Lockdown” plug-in for WordPress. It’s always interesting when “security” software ends up having serious security flaws…. This patch contained several bug fixes. The first bug fix we see in the patch is the inclusion of nonce checking to prevent CSRF. It’s difficult to detect CSRF vulnerabilities by looking at individual function logic and it’s ok if the reader missed these bugs. CSRF token validation should be done at the framework level and including CSRF nonce validation in the logic of every function can quickly become unwieldy. If an application wide CSRF solution cannot be implemented at the framework level,then auditing for CSRF must be done at a function by function level. Personally,I prefer to check for CSRF vulnerabilities by identifying any function that performs a Create,Update,or Delete operation,mapping those functions back to the HTML markup and checking the markup to see if a nonce is passed as part of the POST or GET request. This is of course is done after an extensive audit of the nonce validation code. The bugs that should have been spotted by the spotthevuln reader are the SQL injection and the XSS vulnerabilities in the code. The SQL Injection is pretty straight forward. The “releaseme” POST parameter is taken and is eventually passed directly to a dynamically built SQL statement without any sanitization. The developers fixed the vulnerability by utilizing the WordPress escape logic. Finally,the last line of the code snippet actually contained an XSS vulnerability,echoing a $_SERVER variable without any sanitization. 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
| function print_loginlockdownAdminPage() {
global $wpdb;
$table_name = $wpdb->prefix . "lockdowns";
$loginlockdownAdminOptions = get_loginlockdownOptions();
if (isset($_POST['update_loginlockdownSettings'])) {
+ //wp_nonce check
+ check_admin_referer('login-lockdown_update-options');
if (isset($_POST['ll_max_login_retries'])) {
$loginlockdownAdminOptions['max_login_retries'] = $_POST['ll_max_login_retries'];
}
if (isset($_POST['ll_retries_within'])) {
$loginlockdownAdminOptions['retries_within'] = $_POST['ll_retries_within'];
}
if (isset($_POST['ll_lockout_length'])) {
$loginlockdownAdminOptions['lockout_length'] = $_POST['ll_lockout_length'];
}
if (isset($_POST['ll_lockout_invalid_usernames'])) {
$loginlockdownAdminOptions['lockout_invalid_usernames'] = $_POST['ll_lockout_invalid_usernames'];
}
if (isset($_POST['ll_mask_login_errors'])) {
$loginlockdownAdminOptions['mask_login_errors'] = $_POST['ll_mask_login_errors'];
}
update_option("loginlockdownAdminOptions",$loginlockdownAdminOptions);
?>
<div><p><strong><?php _e("Settings Updated.","loginlockdown");?></strong></p></div>
<?php
}
if (isset($_POST['release_lockdowns'])) {
+ //wp_nonce check
+ check_admin_referer('login-lockdown_release-lockdowns');
if (isset($_POST['releaseme'])) {
$released = $_POST['releaseme'];
foreach ( $released as $release_id ) {
$results = $wpdb->query("UPDATE $table_name SET release_date = now() ".
- "WHERE lockdown_ID = $release_id");
+ "WHERE lockdown_ID = ". $wpdb->escape($release_id) . "");
}
}
update_option("loginlockdownAdminOptions",$loginlockdownAdminOptions);
?>
<div><p><strong><?php _e("Lockdowns Released.","loginlockdown");?></strong></p></div>
<?php
}
$dalist = listLockedDown();
?>
<div>
-<form method="post"action="<?php echo $_SERVER["REQUEST_URI"];?>">
+<form method="post"action="<?php echo esc_attr($_SERVER["REQUEST_URI"]);?>"> |
|
