| 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:The Hacker’s Diet (WordPress Plugin) Fixed in Version:0.9.7b Issue Type:SQL Injection Original Code: Found Here DescriptionThis week’s vulnerability was a SQL injection vulnerability affecting the Hacker’s Diet WordPress plugin. In the vulnerable version,the plugin assigns several variables using values obtained directly from the querystring. The variable assignments are shown below: $weeks = $_GET["weeks"];
$start_date = $_GET["start_date"];
$end_date = $_GET["end_date"];
$goal = $_GET["goal"];
$user_id = $_GET["user"];
$maint_mode = $_GET["maint_mode"];
No sanitization or validation is done before assigning the values. Once the assignments are made,the attacker controlled values are then passed to a dynamic SQL string here resulting in SQL Injection: $query = “select date,weight,trend from “.$table_prefix.”hackdiet_weightlog where wp_id = $user_id and date >\”".date(“Y-m-d”,strtotime(“$weeks weeks ago”)).”\”order by date asc”; $query = “select date,weight,trend from “.$table_prefix.”hackdiet_weightlog where wp_id = $user_id and date >= \”$start_date\”and date <= \”$end_date\”order by date asc”;
The plugin authors patched this vulnerability by validating that the $_GET[“user”] and $_GET[“weeks”] parameters contains only numeric characters. An interesting exercise would be to trace through the code and find where the following variables are being used: $start_date = $_GET["start_date"];
$end_date = $_GET["end_date"];
$goal = $_GET["goal"];
$maint_mode = $_GET["maint_mode"];
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
| <?php
include (dirname(__FILE__)."/jpgraph/jpgraph.php");
include (dirname(__FILE__)."/jpgraph/jpgraph_line.php");
include (dirname(__FILE__)."/jpgraph/jpgraph_scatter.php");
// get our db settings without loading all of wordpress every save
$html = implode('',file("../../../wp-config.php"));
$html = str_replace ("require_once","// ",$html);
$html = str_replace ("<?php","",$html);
$html = str_replace ("?>","",$html);
eval($html);
mysql_connect(DB_HOST,DB_USER,DB_PASSWORD);
mysql_select_db(DB_NAME);
+if (!is_numeric($_GET["user"]) || !is_numeric($_GET["weeks"])) {
+ exit;
+}
$weeks = $_GET["weeks"];
$start_date = $_GET["start_date"];
$end_date = $_GET["end_date"];
$goal = $_GET["goal"];
$user_id = $_GET["user"];
$maint_mode = $_GET["maint_mode"];
if ($weeks) {
- $query = "select date,weight,trend from ".$table_prefix."hackdiet_weightlog where wp_id = $user_id and date >\"".date("Y-m-d",strtotime("$weeks weeks ago"))."\"order by date asc";
+ $query = "select date,weight,trend from ".$table_prefix."hackdiet_weightlog where wp_id = \"". $user_id . "\"and date >\"".date("Y-m-d",strtotime("$weeks weeks ago"))."\"order by date asc";
} else if ($start_date and $end_date) {
- $query = "select date,weight,trend from ".$table_prefix."hackdiet_weightlog where wp_id = $user_id and date >= \"$start_date\"and date <= \"$end_date\"order by date asc";
+ $query = "select date,weight,trend from ".$table_prefix."hackdiet_weightlog where wp_id = \"". $user_id . "\"and date >= \"$start_date\"and date <= \"$end_date\"order by date asc";
}
result = mysql_query($query);
if (mysql_num_rows($result)) {
if (mysql_num_rows($result) == 1) {
// only one day,gotta finagle the display
$row = mysql_fetch_assoc($result);
// fake day before
$weight_data[] = 0;
if ($goal >0) {
$goal_data[] = $goal;
}
$x_data[] = date("n/j",strtotime("yesterday",strtotime($row["date"])));
// data
$weight_data[] = $row["weight"];
if ($goal >0) {
$goal_data[] = $goal;
}
$x_data[] = date("n/j",strtotime($row["date"]));
// fake day after
$weight_data[] = 0;
if ($goal >0) {
$goal_data[] = $goal;
}
$x_data[] = date("n/j",strtotime("tomorrow",strtotime($row["date"])));
} else {
$num_rows = mysql_num_rows($result);
if ($num_rows <= 7 * 2) { // 0-2 weeks
$ticks = "daily";
} else if ($num_rows <= 31 * 4) { // 2 weeks - 4 months
$ticks = "weekly";
} else { // 4 months +
$ticks = "monthly";
}
$count = 1;
while ($row = mysql_fetch_assoc($result)) {
$weight_data[] = $row["weight"];
$trend_data[] = $row["trend"];
if ($goal >0) {
$goal_data[] = $goal;
}
switch ($ticks) {
case "weekly":
if ($count == 1) {
$x_data[] = date("n/j",strtotime($row["date"]));
} else {
$x_data[] = "";
if ($count == 7) {
$count = 0;
}
}
break;
case "monthly":
if (date("j",strtotime($row["date"])) == "1") {
$x_data[] = date("n/j",strtotime($row["date"]));
} else {
$x_data[] = "";
}
break;
case "daily":
default:
$x_data[] = date("n/j",strtotime($row["date"]));
break;
}
$count++;
}
} |
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"]);?>"> |
|
