Vulnerability with highlight

Read me first before posting anywhere!
Subscribe to the feed, available in Image Atom or Image RSS format.
Get Involved
Post Reply
User avatar
psoTFX
Former Team Member
Posts: 7425
Joined: Tue Jul 03, 2001 8:50 pm

Vulnerability with highlight

Post by psoTFX » Fri Nov 22, 2002 12:28 pm

It has been brought to our attention (after the public were informed) that a vulnerability exists within the highlighting code in viewtopic. While this vulnerability is unlikely to affect the majority of people you will find below a relevant (working ... unlike what you may have read on various lists ...) fix below. A new version of phpBB will be released in the near future. The vulnerability will only become apparent if you click (or directly) enter a link with some relevant content (e.g. script tags, etc.) ... so one easy way to prevent problems is "watch what you click!"

I would like to add that ONCE AGAIN we weren't notified and given time to fix this issue. Instead the irresponsible person concerned posted the information (with a fix which won't work) first to our bug tracker and soon after to (what appears) every "security" list he/she could find. These lists are increasingly doing more damage than good IMO ... rarely have we first been contacted by them to check the validity of problems or whether a fix is in place ... totally irresponsible behaviour in todays world.

Fix:

Open viewtopic.php and find:

Code: Select all

// Was a highlight request part of the URI? Yes, this idea was
// taken from vB but we did already have a highlighter in place
// in search itself ... it's just been extended a bit!
//
if ( isset($HTTP_GET_VARS['highlight']) )
{
	$highlight_match = array();

	//
	// Split words and phrases
	//
	$words = explode(' ', trim(urldecode($HTTP_GET_VARS['highlight'])));

	for($i = 0; $i < count($words); $i++)
	{
		if ( trim($words[$i]) != '' )
		{
			$highlight_match[] = '#\b(' . str_replace("*", "([\w]+)?", $words[$i]) . ')\b#is';
		}
	}

	$highlight_active = ( count($highlight_match) ) ? true : false;
}
else
{
	$highlight_active = false;
}
replace with:

Code: Select all

//
// Was a highlight request part of the URI?
//
$highlight_match = $highlight = '';
if (isset($HTTP_GET_VARS['highlight']))
{
	// Split words and phrases
	$words = explode(' ', trim(htmlspecialchars(urldecode($HTTP_GET_VARS['highlight']))));

	for($i = 0; $i < count($words); $i++)
	{
		if ( trim($words[$i]) != '' )
		{
			$highlight_match .= (($highlight_match != '') ? '|' : '') . str_replace('*', '\w*', phpbb_preg_quote($words[$i], '#'));
		}
	}
	unset($words);

	$highlight = urlencode($HTTP_GET_VARS['highlight']);
}
Find:

Code: Select all

//
// If we've got a hightlight set pass it on to pagination, 
// I get annoyed when I lose my highlight after the first page.
//
$pagination = ( $highlight_active ) ? generate_pagination("viewtopic.$phpEx?" . POST_TOPIC_URL . "=$topic_id&postdays=$post_days&postorder=$post_order&highlight=" . $HTTP_GET_VARS['highlight'], $total_replies, $board_config['posts_per_page'], $start) : generate_pagination("viewtopic.$phpEx?" . POST_TOPIC_URL . "=$topic_id&postdays=$post_days&postorder=$post_order", $total_replies, $board_config['posts_per_page'], $start);
replace with:

Code: Select all

//
// If we've got a hightlight set pass it on to pagination,
// I get annoyed when I lose my highlight after the first page.
//
$pagination = ( $highlight_match ) ? generate_pagination("viewtopic.$phpEx?" . POST_TOPIC_URL . "=$topic_id&postdays=$post_days&postorder=$post_order&highlight=$highlight", $total_replies, $board_config['posts_per_page'], $start) : generate_pagination("viewtopic.$phpEx?" . POST_TOPIC_URL . "=$topic_id&postdays=$post_days&postorder=$post_order", $total_replies, $board_config['posts_per_page'], $start);
Find:

Code: Select all

//
// Send vars to template
//
$template->assign_vars(array(
	'FORUM_ID' => $forum_id,
    'FORUM_NAME' => $forum_name,
    'TOPIC_ID' => $topic_id,
    'TOPIC_TITLE' => $topic_title,
	'PAGINATION' => $pagination,
	'PAGE_NUMBER' => sprintf($lang['Page_of'], ( floor( $start / $board_config['posts_per_page'] ) + 1 ), ceil( $total_replies / $board_config['posts_per_page'] )), 

	'POST_IMG' => $post_img,
	'REPLY_IMG' => $reply_img,

	'L_AUTHOR' => $lang['Author'],
	'L_MESSAGE' => $lang['Message'],
	'L_POSTED' => $lang['Posted'], 
	'L_POST_SUBJECT' => $lang['Post_subject'],
	'L_VIEW_NEXT_TOPIC' => $lang['View_next_topic'],
	'L_VIEW_PREVIOUS_TOPIC' => $lang['View_previous_topic'],
	'L_POST_NEW_TOPIC' => $post_alt, 
	'L_POST_REPLY_TOPIC' => $reply_alt, 
	'L_BACK_TO_TOP' => $lang['Back_to_top'],
	'L_DISPLAY_POSTS' => $lang['Display_posts'],
	'L_LOCK_TOPIC' => $lang['Lock_topic'], 
	'L_UNLOCK_TOPIC' => $lang['Unlock_topic'], 
	'L_MOVE_TOPIC' => $lang['Move_topic'], 
	'L_SPLIT_TOPIC' => $lang['Split_topic'], 
	'L_DELETE_TOPIC' => $lang['Delete_topic'], 
	'L_GOTO_PAGE' => $lang['Goto_page'], 

	'S_TOPIC_LINK' => POST_TOPIC_URL, 
	'S_SELECT_POST_DAYS' => $select_post_days,
	'S_SELECT_POST_ORDER' => $select_post_order,
	'S_POST_DAYS_ACTION' => append_sid("viewtopic.$phpEx?" . POST_TOPIC_URL . '=' . $topic_id . "&start=$start"), 
	'S_AUTH_LIST' => $s_auth_can,
	'S_TOPIC_ADMIN' => $topic_mod,
	'S_WATCH_TOPIC' => $s_watching_topic,

	'U_VIEW_TOPIC' => append_sid("viewtopic.$phpEx?" . POST_TOPIC_URL . "=$topic_id&start=$start&postdays=$post_days&postorder=$post_order&highlight=" . $HTTP_GET_VARS['highlight']), 
	'U_VIEW_FORUM' => $view_forum_url,
	'U_VIEW_OLDER_TOPIC' => $view_prev_topic_url,
	'U_VIEW_NEWER_TOPIC' => $view_next_topic_url,
	'U_POST_NEW_TOPIC' => $new_topic_url,
	'U_POST_REPLY_TOPIC' => $reply_topic_url)
);
replace with:

Code: Select all

//
// Send vars to template
//
$template->assign_vars(array(
	'FORUM_ID' => $forum_id,
    'FORUM_NAME' => $forum_name,
    'TOPIC_ID' => $topic_id,
    'TOPIC_TITLE' => $topic_title,
	'PAGINATION' => $pagination,
	'PAGE_NUMBER' => sprintf($lang['Page_of'], ( floor( $start / $board_config['posts_per_page'] ) + 1 ), ceil( $total_replies / $board_config['posts_per_page'] )),

	'POST_IMG' => $post_img,
	'REPLY_IMG' => $reply_img,

	'L_AUTHOR' => $lang['Author'],
	'L_MESSAGE' => $lang['Message'],
	'L_POSTED' => $lang['Posted'],
	'L_POST_SUBJECT' => $lang['Post_subject'],
	'L_VIEW_NEXT_TOPIC' => $lang['View_next_topic'],
	'L_VIEW_PREVIOUS_TOPIC' => $lang['View_previous_topic'],
	'L_POST_NEW_TOPIC' => $post_alt,
	'L_POST_REPLY_TOPIC' => $reply_alt,
	'L_BACK_TO_TOP' => $lang['Back_to_top'],
	'L_DISPLAY_POSTS' => $lang['Display_posts'],
	'L_LOCK_TOPIC' => $lang['Lock_topic'],
	'L_UNLOCK_TOPIC' => $lang['Unlock_topic'],
	'L_MOVE_TOPIC' => $lang['Move_topic'],
	'L_SPLIT_TOPIC' => $lang['Split_topic'],
	'L_DELETE_TOPIC' => $lang['Delete_topic'],
	'L_GOTO_PAGE' => $lang['Goto_page'],

	'S_TOPIC_LINK' => POST_TOPIC_URL,
	'S_SELECT_POST_DAYS' => $select_post_days,
	'S_SELECT_POST_ORDER' => $select_post_order,
	'S_POST_DAYS_ACTION' => append_sid("viewtopic.$phpEx?" . POST_TOPIC_URL . '=' . $topic_id . "&start=$start"),
	'S_AUTH_LIST' => $s_auth_can,
	'S_TOPIC_ADMIN' => $topic_mod,
	'S_WATCH_TOPIC' => $s_watching_topic,

	'U_VIEW_TOPIC' => append_sid("viewtopic.$phpEx?" . POST_TOPIC_URL . "=$topic_id&start=$start&postdays=$post_days&postorder=$post_order&highlight=$highlight"),
	'U_VIEW_FORUM' => $view_forum_url,
	'U_VIEW_OLDER_TOPIC' => $view_prev_topic_url,
	'U_VIEW_NEWER_TOPIC' => $view_next_topic_url,
	'U_POST_NEW_TOPIC' => $new_topic_url,
	'U_POST_REPLY_TOPIC' => $reply_topic_url)
);
Find:

Code: Select all

//
	// Highlight active words (primarily for search)
	//
	if ( $highlight_active )
	{
		if ( preg_match('/<.*>/', $message) )
		{
			$message = preg_replace($highlight_match, '<!-- #sh -->\1<!-- #eh -->', $message);

			$end_html = 0;
			$start_html = 1;
			$temp_message = '';
			$message = ' ' . $message . ' ';

			while( $start_html = strpos($message, '<', $start_html) )
			{
				$grab_length = $start_html - $end_html - 1;
				$temp_message .= substr($message, $end_html + 1, $grab_length);

				if ( $end_html = strpos($message, '>', $start_html) )
				{
					$length = $end_html - $start_html + 1;
					$hold_string = substr($message, $start_html, $length);

					if ( strrpos(' ' . $hold_string, '<') != 1 )
					{
						$end_html = $start_html + 1;
						$end_counter = 1;

						while ( $end_counter && $end_html < strlen($message) )
						{
							if ( substr($message, $end_html, 1) == '>' )
							{
								$end_counter--;
							}
							else if ( substr($message, $end_html, 1) == '<' )
							{
								$end_counter++;
							}

							$end_html++;
						}

						$length = $end_html - $start_html + 1;
						$hold_string = substr($message, $start_html, $length);
						$hold_string = str_replace('<!-- #sh -->', '', $hold_string);
						$hold_string = str_replace('<!-- #eh -->', '', $hold_string);
					}
					else if ( $hold_string == '<!-- #sh -->' )
					{
						$hold_string = str_replace('<!-- #sh -->', '<span style="color:#' . $theme['fontcolor3'] . '"><b>', $hold_string);
					}
					else if ( $hold_string == '<!-- #eh -->' )
					{
						$hold_string = str_replace('<!-- #eh -->', '</b></span>', $hold_string);
					}

					$temp_message .= $hold_string;

					$start_html += $length;
				}
				else
				{
					$start_html = strlen($message);
				}
			}

			$grab_length = strlen($message) - $end_html - 1;
			$temp_message .= substr($message, $end_html + 1, $grab_length);

			$message = trim($temp_message);
		}
		else
		{
			$message = preg_replace($highlight_match, '<span style="color:#' . $theme['fontcolor3'] . '"><b>\1</b></span>', $message);
		}
	}
replace with:

Code: Select all

//
	// Highlight active words (primarily for search)
	//
	if ($highlight_match)
	{
		// This was shamelessly 'borrowed' from volker at multiartstudio dot de
		// via php.net's annotated manual
		$message = str_replace('\"', '"', substr(preg_replace('#(\>(((?>([^><]+|(?R)))*)\<))#se', "preg_replace('#\b(" . $highlight_match . ")\b#i', '<span style=\"color:#" . $theme['fontcolor3'] . "\"><b>\\\\1</b></span>', '\\0')", '>' . $message . '<'), 1, -1));
	}
This fix is already in CVS and will (or should!) cure other reported issues with broken HTML when highlighting some words.
Last edited by psoTFX on Fri Nov 22, 2002 4:21 pm, edited 1 time in total.

Post Reply

Return to “Announcements”