AppSec Blog

Spot the Vuln - Percentage - Cross Site Scripting


Affected Software: Sermon Browser WordPress Plugin

Fixed in Version: .44

Issue Type: Cross Site Scripting

Original Code: Found Here


There is a lot going on here in this code snippet. First, let's talk about the patch. The patch adds a check to ensure the user requesting has the rights to edit a post. The added functionality only displays a link (A HREF) if the user has the correct permissions. Let's hope there are additional checks in place to prevent the execution of this functionality, as opposed to just trying to obscure the link.

Second, there are a few SQL queries. The SQL queries actually seem to be well handled; most values are cast to int, which should work. Of course, Neal Poole and Jacob astutely point out that casts to int cannot always be trusted ( <- see comments section). Abraham Kang also points out that some escaping functions can be defeated by using alternate charsets. The better solution is to use parameterized queries.

What about XSS? There is XSS everywhere on this page. In fact, there is so much XSS I'm not even going to try to list all of the exposures. Instead, I'll focus on two different patterns. Let's start with a classic XSS bug seen in many different PHP products. On line XX, the developer echoes back $_SERVER[?PHP_SELF']. This results in XSS. Many developers think that PHP_SELF will only echo back the current URL path with no user controlled input, but sadly this is not the case. PHP_SELF can almost always be tainted by a user/attacker. This comment on the reserved variables page for PHP sums it up nicely:

Now, looking at lines 30-35, we see the developer uses stripslashes before echoing back a user controlled value. I'm guessing this is because the developer was worried about echoing back data that was stored with a database escaping function. Unfortunately, stripslashes does not prevent XSS. Pretty much all of the echo statements on this page are vulnerable to XSS (unless the data is being stored in an HTML encoded state). The echo statements on line 68 and line 70 are 100% XSS.

From the looks of this code, it's likely the developer doesn't know what XSS is. Any security training should probably first focus on this deficiency. Other code written by this developer should also be audited for XSS.

Developers Solution

<?php ...snip... } elseif (isset($_POST['fetch'])) { // ajax pagination if (function_exists('wp_timezone_override_offset')) wp_timezone_override_offset(); $st = (int) $_POST['fetch'] - 1; if (!empty($_POST['title'])) { $cond = "and m.title LIKE '%" . mysql_real_escape_string($_POST['title']) . "%' "; } else $cond = "; if ($_POST['preacher'] != 0) { $cond .= 'and m.preacher_id = ' . (int) $_POST['preacher'] . ' '; } if ($_POST['series'] != 0) { $cond .= 'and m.series_id = ' . (int) $_POST['series'] . ' '; } $m = $wpdb->get_results("SELECT SQL_CALC_FOUND_ROWS, m.title, m.datetime, as pname, as sname, as ssname FROM {$wpdb->prefix}sb_sermons as m LEFT JOIN {$wpdb->prefix}sb_preachers as p ON m.preacher_id = LEFT JOIN {$wpdb->prefix}sb_services as s ON m.service_id = LEFT JOIN {$wpdb->prefix}sb_series as ss ON m.series_id = WHERE 1=1 {$cond} ORDER BY m.datetime desc, s.time desc LIMIT {$st}, ".sb_get_option('sermons_per_page')); $cnt = $wpdb->get_var("SELECT FOUND_ROWS()"); ?> <?php foreach ($m as $sermon): ?> <tr class="<?php echo ++$i % 2 == 0 ? 'alternate' : " ?>"> <th style="text-align:center" scope="row"><?php echo $sermon->id ?></th> <td><?php echo stripslashes($sermon->title) ?></td> <td><?php echo stripslashes($sermon->pname) ?></td> <td><?php echo ($sermon->datetime == '1970-01-01 00:00:00') ? __('Unknown', $sermon_domain) : strftime('%d %b %y', strtotime($sermon->datetime)); ?></td> <td><?php echo stripslashes($sermon->sname) ?></td> <td><?php echo stripslashes($sermon->ssname) ?></td> <td><?php echo sb_sermon_stats($sermon->id) ?></td> <td style="text-align:center"> -<a href="<?php echo $_SERVER['PHP_SELF']?>?page=sermon-browser/new_sermon.php&mid=<?php echo $sermon->id ?>"><?php _e('Edit', $sermon_domain) ?></a> | <a onclick="return confirm('Are you sure?')" href="<?php echo $_SERVER['PHP_SELF']?>?page=sermon-browser/sermon.php&mid=<?php echo $sermon->id ?>"><?php _e('Delete', $sermon_domain) ?></a> +<?php //Security check +if (current_user_can('edit_posts')) { ?> +<a href="<?php echo $_SERVER['PHP_SELF']?>?page=sermon-browser/new_sermon.php&mid=<?php echo $sermon->id ?>"><?php _e('Edit', $sermon_domain) ?></a> | <a onclick="return confirm('Are you sure?')" href="<?php echo $_SERVER['PHP_SELF']?>?page=sermon-browser/sermon.php&mid=<?php echo $sermon->id ?>"><?php _e('Delete', $sermon_domain); ?></a> | +<?php } ?> +<a href="<?php echo sb_display_url().sb_query_char(true).'sermon_id='.$sermon->id;?>">View</a> </td> </tr> <?php endforeach ?> <script type="text/javascript"> <?php if($cnt<sb_get_option('sermons_per_page') || $cnt <= $st+sb_get_option('sermons_per_page')): ?> jQuery('#right').css('display','none'); <?php elseif($cnt > $st+sb_get_option('sermons_per_page')): ?> jQuery('#right').css('display',"); <?php endif ?> </script> <?php } elseif (isset($_POST['fetchU']) || isset($_POST['fetchL']) || isset($_POST['search'])) { // ajax pagination (uploads) if (isset($_POST['fetchU'])) { $st = (int) $_POST['fetchU'] - 1; $abc = $wpdb->get_results("SELECT f.*, s.title FROM {$wpdb->prefix}sb_stuff AS f LEFT JOIN {$wpdb->prefix}sb_sermons AS s ON f.sermon_id = WHERE f.sermon_id = 0 AND f.type = 'file' ORDER BY LIMIT {$st}, ".sb_get_option('sermons_per_page')); } elseif (isset($_POST['fetchL'])) { $st = (int) $_POST['fetchL'] - 1; $abc = $wpdb->get_results("SELECT f.*, s.title FROM {$wpdb->prefix}sb_stuff AS f LEFT JOIN {$wpdb->prefix}sb_sermons AS s ON f.sermon_id = WHERE f.sermon_id <> 0 AND f.type = 'file' ORDER BY LIMIT {$st}, ".sb_get_option('sermons_per_page')); } else { $s = mysql_real_escape_string($_POST['search']); $abc = $wpdb->get_results("SELECT f.*, s.title FROM {$wpdb->prefix}sb_stuff AS f LEFT JOIN {$wpdb->prefix}sb_sermons AS s ON f.sermon_id = WHERE LIKE '%{$s}%' AND f.type = 'file' ORDER BY;"); } ?> <?php if (count($abc) >= 1): ?> <?php foreach ($abc as $file): ?> <tr class="file <?php echo (++$i % 2 == 0) ? 'alternate' : " ?>" id="<?php echo $_POST['fetchU'] ? " : 's' ?>file<?php echo $file->id ?>"> <th style="text-align:center" scope="row"><?php echo $file->id ?></th> <td id="<?php echo $_POST['fetchU'] ? " : 's' ?><?php echo $file->id ?>"><?php echo substr($file->name, 0, strrpos($file->name, '.')) ?></td> <td style="text-align:center"><?php echo isset($filetypes[substr($file->name, strrpos($file->name, '.') + 1)]['name']) ? $filetypes[substr($file->name, strrpos($file->name, '.') + 1)]['name'] : strtoupper(substr($file->name, strrpos($file->name, '.') + 1)) ?></td> <?php if (!isset($_POST['fetchU'])) { ?><td><?php echo stripslashes($file->title) ?></td><?php } ?> <td style="text-align:center"> <script type="text/javascript" language="javascript"> function deletelinked_<?php echo $file->id;?>(filename, filesermon) { if (confirm('Do you really want to delete '+filename+'?')) { if (filesermon != ") { return confirm('This file is linked to the sermon called ['+filesermon+']. Are you sure you want to delete it?'); } return true; } return false; } </script> <?php if (isset($_POST['fetchU'])) { ?><a id="" href="<?php echo $_SERVER['PHP_SELF']."?page=sermon-browser/new_sermon.php&amp;getid3={$file->id}"; ?>"><?php _e('Create sermon', $sermon_domain) ?></a> | <?php } ?> <a id="link<?php echo $file->id ?>" href="javascript:rename(<?php echo $file->id ?>, '<?php echo $file->name ?>')"><?php _e('Rename', $sermon_domain) ?></a> | <a onclick="return deletelinked_<?php echo $file->id;?>('<?php echo str_replace("'", ", $file->name) ?>', '<?php echo str_replace("'", ", $file->title) ?>');" href="javascript:kill(<?php echo $file->id ?>, '<?php echo $file->name ?>');"><?php _e('Delete', $sermon_domain) ?></a> </td> </tr> <?php endforeach ?> <?php else: ?> <tr> <td><?php _e('No results', $sermon_domain) ?></td> </tr> <?php endif ?> <?php } die(); ?>

Post a Comment


* Indicates a required field.