Vulnerability with highlight

Read me first before posting anywhere!
Subscribe to the feed, available in Image Atom or Image RSS format.

Vulnerability with highlight

Postby 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&amp;postdays=$post_days&amp;postorder=$post_order&amp;highlight=" . $HTTP_GET_VARS['highlight'], $total_replies, $board_config['posts_per_page'], $start) : generate_pagination("viewtopic.$phpEx?" . POST_TOPIC_URL . "=$topic_id&amp;postdays=$post_days&amp;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&amp;postdays=$post_days&amp;postorder=$post_order&amp;highlight=$highlight", $total_replies, $board_config['posts_per_page'], $start) : generate_pagination("viewtopic.$phpEx?" . POST_TOPIC_URL . "=$topic_id&amp;postdays=$post_days&amp;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 . "&amp;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&amp;start=$start&amp;postdays=$post_days&amp;postorder=$post_order&amp;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 . "&amp;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&amp;start=$start&amp;postdays=$post_days&amp;postorder=$post_order&amp;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.
User avatar
psoTFX
Former Team Member
 
Posts: 7426
Joined: Tue Jul 03, 2001 8:50 pm

Return to Announcements

Who is online

Users browsing this forum: jpcatherine, NaezoRose and 25 guests