Skip to content

Commit

Permalink
Fixed stored XSS vulnerabilities caused by unescaped content titles in:
Browse files Browse the repository at this point in the history 
- Content navigation pane.
- Delete content page.
- Edit content page.
- Arrange content page.
- Course page headers and footers.

Also some minor refactoring in ContentManager.class.php: Cleaned up redundant variables and function calls.
  • Loading branch information
Per Wammer committed Dec 31, 2016
1 parent cd797c1 commit a36b166
Show file tree
Hide file tree
Showing 26 changed files with 92 additions and 94 deletions.
38 changes: 18 additions & 20 deletions include/classes/ContentManager.class.php
View file
Expand Up @@ -969,6 +969,7 @@ function printMenu($parent_id, $depth, $path, $children, $truncate, $ignore_stat
//tests do not have content id
$content['content_id'] = isset($content['content_id']) ? $content['content_id'] : '';
$content['parent_content_id'] = $parent_id;
$content['title'] = htmlspecialchars($content['title']);

if (!$ignore_state) {
$link .= '<a name="menu'.$content['content_id'].'"></a>';
Expand All @@ -986,16 +987,14 @@ function printMenu($parent_id, $depth, $path, $children, $truncate, $ignore_stat
//content test extension @harris
//if this is a test link.
if (isset($content['test_id'])){
$title_n_alt = ContentManager::cleanOutput($content['title']);
//$in_link = 'mods/_standard/tests/test_intro.php?tid='.$content['test_id'].SEP.'in_cid='.$content['parent_content_id'];
$in_link = $_base_href.'mods/_standard/tests/test_intro.php?tid='.$content['test_id'].SEP.'in_cid='.$content['parent_content_id'];
$img_link = ' <img src="'.$_base_path.'images/check.gif" title="'.$title_n_alt.'" alt="'.$title_n_alt.'" />';
$img_link = ' <img src="'.$_base_path.'images/check.gif" title="'.$content['title'].'" alt="'.$content['title'].'" />';
} else {
$in_link = 'content.php?cid='.$content['content_id'];
$img_link = '';
}

$full_title = ContentManager::cleanOutput($content['title']);
//$link .= $img_link . ' <a href="'.$_base_path.htmlentities_utf8(url_rewrite($in_link)).'" title="';
$link .= $img_link . ' <a href="'.$_base_path.htmlentities_utf8($in_link).'" title="';
$base_title_length = 29;
Expand All @@ -1010,13 +1009,13 @@ function printMenu($parent_id, $depth, $path, $children, $truncate, $ignore_stat
}

if (isset($content['test_id'])) {
$link .= ContentManager::cleanOutput($content['title']);
$link .= $content['title'];
} else {
$link .= '<span class="inlineEdits" id="menu-'.$content['content_id'].'" title="'.ContentManager::cleanOutput($full_title).'">';
$link .= '<span class="inlineEdits" id="menu-'.$content['content_id'].'" title="'.$content['title'].'">';
if($_SESSION['prefs']['PREF_NUMBERING']){
$link .= $path.$counter;
}
$link .= '&nbsp;'.ContentManager::cleanOutput($content['title']).'</span>';
$link .= '&nbsp;'.$content['title'].'</span>';
}

$link .= '</a>';
Expand All @@ -1043,7 +1042,6 @@ function printMenu($parent_id, $depth, $path, $children, $truncate, $ignore_stat

if ($content['content_type'] == CONTENT_TYPE_CONTENT || $content['content_type'] == CONTENT_TYPE_WEBLINK)
{ // current content page
$full_title = $content['title'];
$link .= '<a href="'.$_my_uri.'"><img src="'.$_base_path.'images/clr.gif" alt="'._AT('you_are_here').': ';
if($_SESSION['prefs']['PREF_NUMBERING']){
$link .= $path.$counter;
Expand All @@ -1052,7 +1050,7 @@ function printMenu($parent_id, $depth, $path, $children, $truncate, $ignore_stat
if ($truncate && ($strlen($content['title']) > ($base_title_length-$depth*4)) ) {
$content['title'] = htmlspecialchars(rtrim($substr(htmlspecialchars_decode($content['title']), 0, ($base_title_length-$depth*4)-4))).'...';
}
$link .= '<a name="menu'.$content['content_id'].'"></a><span class="inlineEdits" id="menu-'.$content['content_id'].'" title="'.$full_title.'">';
$link .= '<a name="menu'.$content['content_id'].'"></a><span class="inlineEdits" id="menu-'.$content['content_id'].'" title="'.$content['title'].'">';
if($_SESSION['prefs']['PREF_NUMBERING']){
$link .= $path.$counter;
}
Expand All @@ -1065,9 +1063,8 @@ function printMenu($parent_id, $depth, $path, $children, $truncate, $ignore_stat
}
else
{ // nodes with content type "CONTENT_TYPE_FOLDER"
$full_title = ContentManager::cleanOutput($content['title']);
if (authenticate(AT_PRIV_CONTENT, AT_PRIV_RETURN) && !is_mobile_device()) {
$link .= '<a href="'.$_base_path."mods/_core/editor/edit_content_folder.php?cid=".$content['content_id'].'" title="'.ContentManager::cleanOutput($full_title). _AT('click_edit').'">'."\n";
$link .= '<a href="'.$_base_path."mods/_core/editor/edit_content_folder.php?cid=".$content['content_id'].'" title="'.$content['title']. _AT('click_edit').'">'."\n";
}
else {
$link .= '<span style="cursor:pointer" onclick="javascript: ATutor.course.toggleFolder(\''.$content['content_id'].$from.'\', \''._AT('expand').'\', \''._AT('collapse').'\', '.$this->course_id.'); ">'."\n";
Expand All @@ -1077,13 +1074,13 @@ function printMenu($parent_id, $depth, $path, $children, $truncate, $ignore_stat
$content['title'] = stripslashes(htmlspecialchars(rtrim($substr(htmlspecialchars_decode($content['title']), 0, ($base_title_length-$depth*4)-4)))).'...';
}
if (isset($content['test_id']))
$link .= ContentManager::cleanOutput($content['title']);
$link .= $content['title'];
else
$link .= '<span class="inlineEdits" id="menu-'.$content['content_id'].'" title="'.ContentManager::cleanOutput($full_title).'">';
$link .= '<span class="inlineEdits" id="menu-'.$content['content_id'].'" title="'.$content['title'].'">';
if($_SESSION['prefs']['PREF_NUMBERING']){
$link .= $path.$counter;
}
$link .= '&nbsp;'.ContentManager::cleanOutput($content['title']).'</span>';
$link .= '&nbsp;'.$content['title'].'</span>';

if (authenticate(AT_PRIV_CONTENT, AT_PRIV_RETURN) && !is_mobile_device()) {
$link .= '</a>'."\n";
Expand Down Expand Up @@ -1142,7 +1139,7 @@ function printMenu($parent_id, $depth, $path, $children, $truncate, $ignore_stat

} else {
echo '<a href="'.$_my_uri.'collapse='.$content['content_id'].'">'."\n";
echo '<img src="'.AT_BASE_HREF.$this->tree_collapse_icon.'" id="tree_icon'.$content['content_id'].$from.'" alt="'._AT('collapse').'" width="16" height="16" title="'._AT('collapse').' '.ContentManager::cleanOutput($content['title']).'" class="img-size-tree" onclick="javascript: ATutor.course.toggleFolder(\''.$content['content_id'].$from.'\', \''._AT('expand').'\', \''._AT('collapse').'\', '.$this->course_id.'); " />'."\n";
echo '<img src="'.AT_BASE_HREF.$this->tree_collapse_icon.'" id="tree_icon'.$content['content_id'].$from.'" alt="'._AT('collapse').'" width="16" height="16" title="'._AT('collapse').' '.$content['title'].'" class="img-size-tree" onclick="javascript: ATutor.course.toggleFolder(\''.$content['content_id'].$from.'\', \''._AT('expand').'\', \''._AT('collapse').'\', '.$this->course_id.'); " />'."\n";
echo '</a>'."\n";
}
} else {
Expand All @@ -1151,7 +1148,7 @@ function printMenu($parent_id, $depth, $path, $children, $truncate, $ignore_stat

} else {
echo '<a href="'.$_my_uri.'expand='.$content['content_id'].'">'."\n";
echo '<img src="'.AT_BASE_HREF.$this->tree_expand_icon.'" id="tree_icon'.$content['content_id'].$from.'" alt="'._AT('expand').'" width="16" height="16" title="'._AT('expand').' '.ContentManager::cleanOutput($content['title']).'" class="img-size-tree" onclick="javascript: ATutor.course.toggleFolder(\''.$content['content_id'].$from.'\', \''._AT('expand').'\', \''._AT('collapse').'\', '.$this->course_id.'); " />';
echo '<img src="'.AT_BASE_HREF.$this->tree_expand_icon.'" id="tree_icon'.$content['content_id'].$from.'" alt="'._AT('expand').'" width="16" height="16" title="'._AT('expand').' '.$content['title'].'" class="img-size-tree" onclick="javascript: ATutor.course.toggleFolder(\''.$content['content_id'].$from.'\', \''._AT('expand').'\', \''._AT('collapse').'\', '.$this->course_id.'); " />';
echo '</a>'."\n";
}
}
Expand Down Expand Up @@ -1181,7 +1178,7 @@ function printMenu($parent_id, $depth, $path, $children, $truncate, $ignore_stat
}


echo htmlspecialchars_decode($link);
echo $link;

echo "\n<br /></span>\n\n";

Expand Down Expand Up @@ -1230,6 +1227,7 @@ function printActionMenu($menu, $parent_id, $depth, $path, $children, $print_typ
}

$link = $buttons = '';
$content['title'] = htmlspecialchars($content['title']);

echo '<tr>'."\n";

Expand All @@ -1244,17 +1242,17 @@ function printActionMenu($menu, $parent_id, $depth, $path, $children, $print_typ

$buttons = '<td>'."\n".
' <small>'."\n".
' <input type="image" name="move['.$parent_id.'_'.$content['ordering'].']" src="'.$_base_path.'images/before.gif" alt="'._AT('before_topic', ContentManager::cleanOutput($content['title'])).'" title="'._AT('before_topic', ContentManager::cleanOutput($content['title'])).'" style="height:1.5em; width:1.9em;" />'."\n";
' <input type="image" name="move['.$parent_id.'_'.$content['ordering'].']" src="'.$_base_path.'images/before.gif" alt="'._AT('before_topic', $content['title']).'" title="'._AT('before_topic', $content['title']).'" style="height:1.5em; width:1.9em;" />'."\n";

if ($current_num + 1 == count($top_level))
$buttons .= ' <input type="image" name="move['.$parent_id.'_'.($content['ordering']+1).']" src="'.$_base_path.'images/after.gif" alt="'._AT('after_topic', ContentManager::cleanOutput($content['title'])).'" title="'._AT('after_topic', ContentManager::cleanOutput($content['title'])).'" style="height:1.5em; width:1.9em;" />'."\n";
$buttons .= ' <input type="image" name="move['.$parent_id.'_'.($content['ordering']+1).']" src="'.$_base_path.'images/after.gif" alt="'._AT('after_topic', $content['title']).'" title="'._AT('after_topic', $content['title']).'" style="height:1.5em; width:1.9em;" />'."\n";

$buttons .= ' </small>'."\n".
'</td>'."\n".
'<td>';

if ($content['content_type'] == CONTENT_TYPE_FOLDER)
$buttons .= '<input type="image" name="move['.$content['content_id'].'_1]" src="'.$_base_path.'images/child_of.gif" style="height:1.25em; width:1.7em;" alt="'._AT('child_of', ContentManager::cleanOutput($content['title'])).'" title="'._AT('child_of', ContentManager::cleanOutput($content['title'])).'" />';
$buttons .= '<input type="image" name="move['.$content['content_id'].'_1]" src="'.$_base_path.'images/child_of.gif" style="height:1.25em; width:1.7em;" alt="'._AT('child_of', $content['title']).'" title="'._AT('child_of', $content['title']).'" />';
else
$buttons .= '&nbsp;';

Expand All @@ -1279,7 +1277,7 @@ function printActionMenu($menu, $parent_id, $depth, $path, $children, $print_typ
{
$link .= '<img src="'.$_base_path.'images/folder.gif" />';
}
$link .= '&nbsp;<label for="r'.$content['content_id'].'">'.ContentManager::cleanOutput($content['title']).'</label>'."\n";
$link .= '&nbsp;<label for="r'.$content['content_id'].'">'.$content['title'].'</label>'."\n";

if ( is_array($menu[$content['content_id']]) && !empty($menu[$content['content_id']]) ) {
/* has children */
Expand Down
2 changes: 1 addition & 1 deletion mods/_core/editor/delete_content.php
View file
Expand Up @@ -68,7 +68,7 @@
$rows_content = queryDB($sql, array(TABLE_PREFIX, $hidden_vars['cid']));

foreach($rows_content as $row){
$title = $row['title'];
$title = htmlspecialchars($row['title']);
}

$msg->addConfirm(array('DELETE', $title), $hidden_vars);
Expand Down
2 changes: 1 addition & 1 deletion mods/_core/editor/editor_tabs/edit.inc.php
View file
Expand Up @@ -26,7 +26,7 @@
<div class="row">
<span>
<span class="required" title="<?php echo _AT('required_field'); ?>">*</span><label for="ctitle"><strong><?php echo _AT('title'); ?></strong></label>
<input type="text" name="title" id="ctitle" size="60" class="formfield" value="<?php echo ContentManager::cleanOutput($_POST['title']); ?>" />
<input type="text" name="title" id="ctitle" size="60" class="formfield" value="<?php echo htmlspecialchars($_POST['title']); ?>" />
</span>
<br /> <span class="nowrap">
<label for="formatting_radios"><span class="required" title="<?php echo _AT('required_field'); ?>">*</span><strong><?php echo _AT('formatting'); ?></strong></label>
Expand Down
6 changes: 3 additions & 3 deletions themes/atspaces/include/footer.tmpl.php
View file
Expand Up @@ -10,13 +10,13 @@
<div class="sequence-links">
<?php if (isset($_SESSION["prefs"]["PREF_SHOW_NEXT_PREVIOUS_BUTTONS"])) { ?>
<?php if (isset($this->sequence_links['resume'])): ?>
<a style="color:white;" href="<?php echo $this->sequence_links['resume']['url']; ?>" accesskey="."><img src="<?php echo $this->img; ?>resume.png" border="0" title="<?php echo _AT('resume').': '.$this->sequence_links['resume']['title']; ?> Alt+." alt="<?php echo $this->sequence_links['resume']['title']; ?> Alt+." class="img1616"/></a>
<a style="color:white;" href="<?php echo $this->sequence_links['resume']['url']; ?>" accesskey="."><img src="<?php echo $this->img; ?>resume.png" border="0" title="<?php echo _AT('resume').': '.htmlspecialchars($this->sequence_links['resume']['title']); ?> Alt+." alt="<?php echo htmlspecialchars($this->sequence_links['resume']['title']); ?> Alt+." class="img1616"/></a>
<?php else:
if (isset($this->sequence_links['previous'])): ?>
<a href="<?php echo $this->sequence_links['previous']['url']; ?>" title="<?php echo _AT('previous_topic').': '. $this->sequence_links['previous']['title']; ?> Alt+," accesskey=","><img src="<?php echo $this->img; ?>previous.png" border="0" alt="<?php echo _AT('previous_topic').': '. $this->sequence_links['previous']['title']; ?> Alt+," class="img1616" /></a>
<a href="<?php echo $this->sequence_links['previous']['url']; ?>" title="<?php echo _AT('previous_topic').': '. htmlspecialchars($this->sequence_links['previous']['title']); ?> Alt+," accesskey=","><img src="<?php echo $this->img; ?>previous.png" border="0" alt="<?php echo _AT('previous_topic').': '. htmlspecialchars($this->sequence_links['previous']['title']); ?> Alt+," class="img1616" /></a>
<?php endif;
if (isset($this->sequence_links['next'])): ?>
<a href="<?php echo $this->sequence_links['next']['url']; ?>" title="<?php echo _AT('next_topic').': '.$this->sequence_links['next']['title']; ?> Alt+." accesskey="."><img src="<?php echo $this->img; ?>next.png" border="0" alt="<?php echo _AT('next_topic').': '.$this->sequence_links['next']['title']; ?> Alt+." class="img1616" /></a>
<a href="<?php echo $this->sequence_links['next']['url']; ?>" title="<?php echo _AT('next_topic').': '.htmlspecialchars($this->sequence_links['next']['title']); ?> Alt+." accesskey="."><img src="<?php echo $this->img; ?>next.png" border="0" alt="<?php echo _AT('next_topic').': '.htmlspecialchars($this->sequence_links['next']['title']); ?> Alt+." class="img1616" /></a>
<?php endif; ?>
<?php endif; ?>
<?php } ?>
Expand Down
6 changes: 3 additions & 3 deletions themes/atspaces/include/header.tmpl.php
View file
Expand Up @@ -348,13 +348,13 @@
<div class="sequence-links">
<?php if ($_SESSION["prefs"]["PREF_SHOW_NEXT_PREVIOUS_BUTTONS"]) { ?>
<?php if ($this->sequence_links['resume']): ?>
<a style="color:white;" href="<?php echo $this->sequence_links['resume']['url']; ?>" accesskey="."><img src="<?php echo $this->img; ?>resume.png" title="<?php echo _AT('resume').': '.$this->sequence_links['resume']['title']; ?> Alt+." alt="<?php echo $this->sequence_links['resume']['title']; ?> Alt+." class="img1616" /></a>
<a style="color:white;" href="<?php echo $this->sequence_links['resume']['url']; ?>" accesskey="."><img src="<?php echo $this->img; ?>resume.png" title="<?php echo _AT('resume').': '.htmlspecialchars($this->sequence_links['resume']['title']); ?> Alt+." alt="<?php echo htmlspecialchars($this->sequence_links['resume']['title']); ?> Alt+." class="img1616" /></a>
<?php else:
if ($this->sequence_links['previous']): ?>
<a href="<?php echo $this->sequence_links['previous']['url']; ?>" title="<?php echo _AT('previous_topic').': '. $this->sequence_links['previous']['title']; ?> Alt+," accesskey=","><img src="<?php echo $this->img; ?>previous.png" alt="<?php echo _AT('previous_topic').': '. $this->sequence_links['previous']['title']; ?> Alt+," class="img1616" /></a>
<a href="<?php echo $this->sequence_links['previous']['url']; ?>" title="<?php echo _AT('previous_topic').': '. htmlspecialchars($this->sequence_links['previous']['title']); ?> Alt+," accesskey=","><img src="<?php echo $this->img; ?>previous.png" alt="<?php echo _AT('previous_topic').': '. htmlspecialchars($this->sequence_links['previous']['title']); ?> Alt+," class="img1616" /></a>
<?php endif;
if ($this->sequence_links['next']): ?>
<a href="<?php echo $this->sequence_links['next']['url']; ?>" title="<?php echo _AT('next_topic').': '.$this->sequence_links['next']['title']; ?> Alt+." accesskey="."><img src="<?php echo $this->img; ?>next.png" alt="<?php echo _AT('next_topic').': '.$this->sequence_links['next']['title']; ?> Alt+." class="img1616" /></a>
<a href="<?php echo $this->sequence_links['next']['url']; ?>" title="<?php echo _AT('next_topic').': '.htmlspecialchars($this->sequence_links['next']['title']); ?> Alt+." accesskey="."><img src="<?php echo $this->img; ?>next.png" alt="<?php echo _AT('next_topic').': '.htmlspecialchars($this->sequence_links['next']['title']); ?> Alt+." class="img1616" /></a>
<?php endif; ?>
<?php endif; ?>
<?php } ?>
Expand Down
6 changes: 3 additions & 3 deletions themes/blumin/include/footer.tmpl.php
View file
Expand Up @@ -10,13 +10,13 @@
<div class="sequence-links">
<?php if (isset($_SESSION["prefs"]["PREF_SHOW_NEXT_PREVIOUS_BUTTONS"])) { ?>
<?php if (isset($this->sequence_links['resume'])): ?>
<a style="color:white;" href="<?php echo $this->sequence_links['resume']['url']; ?>" accesskey="."><img src="<?php echo $this->img; ?>resume.png" border="0" title="<?php echo _AT('resume').': '.$this->sequence_links['resume']['title']; ?> Alt+." alt="<?php echo $this->sequence_links['resume']['title']; ?> Alt+." class="img1616"/></a>
<a style="color:white;" href="<?php echo $this->sequence_links['resume']['url']; ?>" accesskey="."><img src="<?php echo $this->img; ?>resume.png" border="0" title="<?php echo _AT('resume').': '.htmlspecialchars($this->sequence_links['resume']['title']); ?> Alt+." alt="<?php echo htmlspecialchars($this->sequence_links['resume']['title']); ?> Alt+." class="img1616"/></a>
<?php else:
if (isset($this->sequence_links['previous'])): ?>
<a href="<?php echo $this->sequence_links['previous']['url']; ?>" title="<?php echo _AT('previous_topic').': '. $this->sequence_links['previous']['title']; ?> Alt+," accesskey=","><img src="<?php echo $this->img; ?>previous.png" border="0" alt="<?php echo _AT('previous_topic').': '. $this->sequence_links['previous']['title']; ?> Alt+," class="img1616" /></a>
<a href="<?php echo $this->sequence_links['previous']['url']; ?>" title="<?php echo _AT('previous_topic').': '. htmlspecialchars($this->sequence_links['previous']['title']); ?> Alt+," accesskey=","><img src="<?php echo $this->img; ?>previous.png" border="0" alt="<?php echo _AT('previous_topic').': '. htmlspecialchars($this->sequence_links['previous']['title']); ?> Alt+," class="img1616" /></a>
<?php endif;
if (isset($this->sequence_links['next'])): ?>
<a href="<?php echo $this->sequence_links['next']['url']; ?>" title="<?php echo _AT('next_topic').': '.$this->sequence_links['next']['title']; ?> Alt+." accesskey="."><img src="<?php echo $this->img; ?>next.png" border="0" alt="<?php echo _AT('next_topic').': '.$this->sequence_links['next']['title']; ?> Alt+." class="img1616" /></a>
<a href="<?php echo $this->sequence_links['next']['url']; ?>" title="<?php echo _AT('next_topic').': '.htmlspecialchars($this->sequence_links['next']['title']); ?> Alt+." accesskey="."><img src="<?php echo $this->img; ?>next.png" border="0" alt="<?php echo _AT('next_topic').': '.htmlspecialchars($this->sequence_links['next']['title']); ?> Alt+." class="img1616" /></a>
<?php endif; ?>
<?php endif; ?>
<?php } ?>
Expand Down

0 comments on commit a36b166

Please sign in to comment.