Spot The Vuln – Notes – SQL Injection

Details

Affected Software: Sermon Browser WordPress Plugin

Fixed in Version: .44

Issue Type: Cross Site Scripting

Original Code: Found Here

Details

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

[sourcecode language=”diff” highlight=”23,24″]
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…
?>
[/sourcecode]

Published by

Billy Rios

Billy currently works for Google, a small technology company headquartered in Mountain. Before Google, Billy was a Security Program Manager at Microsoft where he helped secure several high profile software projects including Internet Explorer. Prior to his roles at Google and Microsoft, Billy was a penetration tester, testing the defenses of various companies in the Fortune 500. Billy has spoken at numerous security conferences including: Blackhat briefings, Bluehat, RSA and DEFCON. Billy holds a Bachelors degree in Business Administration, Master of Science degree in Information Systems, and a Master of Business Administration.

Leave a Reply

Your email address will not be published. Required fields are marked *