AppSec Blog

Spot the Vuln - Price - Cross Site Scripting


Affected Software: PunBB

Fixed in Version: 2.1

Issue Type: Cross Site Scripting (XSS)

Original Code: Found Here


This week's vulnerability was a XSS bug in PunBB. PunBB was taking an un-trusted value directly from the POST parameter ($_POST[?prune_sticky']) and echoing the un-trusted value directly into a value attribute for a hidden form input field. You can see the XSS bug in line 98. This echoing of un-trusted input results in XSS.

The PunBB developers did something I really like here. Instead of fixing the single instance of XSS and moving on, the PunBB developers went a step further and hardened the use of $_POST[?prune_sticky']. Instead of allowing users/attacker to provide arbitrary values for $_POST['prune_sticky'] they restricted the acceptable values to 1 or 0. You can see this fix in line 11. This is a perfect example of root cause analysis in action. The PunBB developers took a few minutes to understand how the application uses $_POST[? prune_sticky'] and adjusted the application behavior to protect against other attacks while being transparent to the user. This patch submitted by the PunBB developers goes a long way in protecting their customers and is a great example of being smart about security fixes.

Developers Solution

<?php ... <snip> ... if (isset($_GET['action']) || isset($_POST['prune']) || isset($_POST['prune_comply'])) { if (isset($_POST['prune_comply'])) { confirm_referrer('admin_prune.php'); $prune_from = $_POST['prune_from']; +$prune_sticky = isset($_POST['prune_sticky']) ? '1' : '0'; $prune_days = intval($_POST['prune_days']); $prune_date = ($prune_days) ? time() - ($prune_days*86400) : -1; @set_time_limit(0); if ($prune_from == 'all') { $result = $db->query('SELECT id FROM '.$db->prefix.'forums') or error('Unable to fetch forum list', __FILE__, __LINE__, $db->error()); $num_forums = $db->num_rows($result); for ($i = 0; $i < $num_forums; ++$i) { $fid = $db->result($result, $i); -prune($fid, $_POST['prune_sticky'], $prune_date); +prune($fid, $prune_sticky, $prune_date); update_forum($fid); } } else { $prune_from = intval($prune_from); -prune($prune_from, $_POST['prune_sticky'], $prune_date); +prune($fid, $prune_sticky, $prune_date); update_forum($prune_from); } // Locate any "orphaned redirect topics" and delete them $result = $db->query('SELECT FROM '.$db->prefix.'topics AS t1 LEFT JOIN '.$db->prefix.'topics AS t2 ON WHERE IS NULL AND t1.moved_to IS NOT NULL') or error('Unable to fetch redirect topics', __FILE__, __LINE__, $db->error()); $num_orphans = $db->num_rows($result); if ($num_orphans) { for ($i = 0; $i < $num_orphans; ++$i) $orphans[] = $db->result($result, $i); $db->query('DELETE FROM '.$db->prefix.'topics WHERE id IN('.implode(',', $orphans).')') or error('Unable to delete redirect topics', __FILE__, __LINE__, $db->error()); } redirect('admin_prune.php', 'Posts pruned. Redirecting &hellip;'); } $prune_days = $_POST['req_prune_days']; if (!@preg_match('#^\d+$#', $prune_days)) message('Days to prune must be a positive integer.'); $prune_date = time() - ($prune_days*86400); $prune_from = $_POST['prune_from']; // Concatenate together the query for counting number or topics to prune $sql = 'SELECT COUNT(id) FROM '.$db->prefix.'topics WHERE last_post<'.$prune_date.' AND moved_to IS NULL'; -if ($_POST['prune_sticky'] == '0') +if (!$prune_sticky) $sql .= ' AND sticky=\'0\"; if ($prune_from != 'all') { $prune_from = intval($prune_from); $sql .= ' AND forum_id='.$prune_from; // Fetch the forum name (just for cosmetic reasons) $result = $db->query('SELECT forum_name FROM '.$db->prefix.'forums WHERE id='.$prune_from) or error('Unable to fetch forum name', __FILE__, __LINE__, $db->error()); $forum = '"'.pun_htmlspecialchars($db->result($result)).'"'; } else $forum = 'all forums'; $result = $db->query($sql) or error('Unable to fetch topic prune count', __FILE__, __LINE__, $db->error()); $num_topics = $db->result($result); if (!$num_topics) message('There are no topics that are '.$prune_days.' days old. Please decrease the value of "Days old" and try again.'); $page_title = pun_htmlspecialchars($pun_config['o_board_title']).' / Admin / Prune'; require PUN_ROOT.'header.php'; generate_admin_menu('prune'); ?> <div class="blockform"> <h2><span>Prune</span></h2> <div class="box"> <form method="post" action="admin_prune.php?action=foo"> <div class="inform"> <input type="hidden" name="prune_days" value="<?php echo $prune_days ?>" /> -<input type="hidden" name="prune_sticky" value="<?php echo $_POST['prune_sticky'] ?>" /> +<input type="hidden" name="prune_sticky" value="<?php echo $prune_sticky ?>" /> <input type="hidden" name="prune_from" value="<?php echo $prune_from ?>" /> <fieldset> <legend>Confirm prune posts</legend> <div class="infldset"> <p>Are you sure that you want to prune all topics older than <?php echo $prune_days ?> days from <?php echo $forum ?>? (<?php echo $num_topics ?> topics)</p> <p>WARNING! Pruning posts deletes them permanently.</p> </div> </fieldset> </div> <p><input type="submit" name="prune_comply" value="Prune" /><a href="javascript:history.go(-1)">Go back</a></p> </form> </div> </div> <div class="clearer"></div> </div> ... <snip> ...

Post a Comment


* Indicates a required field.