AppSec Blog

Spot The Vuln - Notes - SQL Injection


Affected Software: Sermon Browser WordPress Plugin

Fixed in Version: .44

Issue Type: Cross Site Scripting

Original Code: Found Here


There are a couple of different issues here, but let's focus on what the developers patched. On line 27, the developer uses the $_GET[?getid3'] value to build a dynamic SQL statement. This is classic SQL injection. The patch seems straight forward, escape the $_GET[?getid3'] value before using it in the SQL statement. Normally, SQL injection involves breaking out of a predefined SQL statement by closing off a quoted string and injecting your own SQL statement. Most escaping functions escape quotes and other special characters so that an attacker cannot escape out of a quoted string. There is a problem in this patch though. The tainted value is NOT enclosed within quotes, so the attacker does not need to escape out of a quoted string. The attacker is free to build a SQL injection payload as long as the payload doesn't contain any special characters. So, despite escaping the $_GET[?getid3'] value, SQL injection is still possible.

Anyone spot the unpatched XSS?

Developers Solution

query("DELETE FROM {$wpdb->prefix}sb_sermons_tags WHERE sermon_id = $id;"); foreach ($tags as $tag) { $clean_tag = trim(mysql_real_escape_string($tag)); $existing_id = $wpdb->get_var("SELECT id FROM {$wpdb->prefix}sb_tags WHERE name='$clean_tag'"); if (is_null($existing_id)) { $wpdb->query("INSERT INTO {$wpdb->prefix}sb_tags VALUES (null, '$clean_tag')"); $existing_id = $wpdb->insert_id; } $wpdb->query("INSERT INTO {$wpdb->prefix}sb_sermons_tags VALUES (null, $id, $existing_id)"); } sb_delete_unused_tags(); // everything is fine, get out of here! if(!isset($error)) { sb_ping_gallery(); echo "document.location = '".$_SERVER['PHP_SELF']."?page=sermon-browser/sermon.php&saved=true';"; die(); } } $id3_tags = array(); if (isset($_GET['getid3'])) { require_once('getid3/getid3.php'); -$file_data = $wpdb->get_row("SELECT name, type FROM {$wpdb->prefix}sb_stuff WHERE id = ".$_GET['getid3']); +$file_data = $wpdb->get_row("SELECT name, type FROM {$wpdb->prefix}sb_stuff WHERE id = ".$wpdb->escape($_GET['getid3'])); if ($file_data !== NULL) { $getID3 = new getID3; if ($file_data->type == 'url') { $filename = substr($file_data->name, strrpos ($file_data->name, '/')+1); $sermonUploadDir = SB_ABSPATH.sb_get_option('upload_dir'); $tempfilename = $sermonUploadDir.preg_replace('/([ ])/e', 'chr(rand(97,122))', '').'.mp3′; if ($tempfile = @fopen($tempfilename, 'wb')) if ($remote_file = @fopen($file_data->name, 'r')) { $remote_contents = "; while (!feof($remote_file)) { $remote_contents .= fread($remote_file, 8192); if (strlen($remote_contents) > 65536) break; } fwrite($tempfile, $remote_contents); fclose($remote_file); fclose($tempfile); $id3_raw_tags = $getID3->analyze(realpath($tempfilename)); unlink ($tempfilename); } } else { $filename = $file_data->name; $id3_raw_tags = $getID3->analyze(realpath(SB_ABSPATH.sb_get_option('upload_dir').$filename)); } if (!isset($id3_raw_tags['tags'])) { echo '

'.__('No ID3 tags found.', $sermon_domain); if ($file_data->type == 'url') echo ' Remote files must have id3v2 tags.'; echo '
'; } ...snip... ?>

Post a Comment


* Indicates a required field.