Details
Affected Software:Login LockDown for WordPress
Fixed in Version: 1.5
Issue Type:SQL Injection
Original Code: Found Here
Description
This 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 Solution
1 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"]);?>"> |