Ticket #86 (closed defect: fixed)

Opened 5 years ago

Last modified 4 years ago

TextSearch - Advanced search results reveal confidential info

Reported by: dartar Owned by: DotMG
Priority: normal Milestone: 1.1.6.2
Component: actions Version: 1.1.6.1
Severity: major Keywords: Search Text Security Confidential
Cc:

Description (last modified by DotMG) (diff)

(reported by PolVazo - patch submitted by QualousHere) Results should be hidden or not shown if the user doesn't have read access to page IMHO

Solution

(based on 1.1.6.0)

  • Changes in actions/textsearch.php

original: 

if ($phrase = $_REQUEST["phrase"])
{
	$phrase = stripslashes($phrase); 
	$results = $this->FullTextSearch($phrase);
	print("<br />");
	$total_results = count($results);
	$match_str = $total_results <> 1 ? " matches" : " match";
	print("Search results: <strong>".$total_results.$match_str."</strong> for <strong>$phrase</strong><br /><br />\n");
	if ($results) 
	{
		foreach ($results as $i => $page)
		{
			print(($i+1).". ".$this->Link($page["tag"])."<br />\n");
		}
		$phrase = urlencode($phrase); 
		print("<br />Not sure which page to choose?<br />Try the <a href=\"".$this->href("", "TextSearchExpanded", "phrase=$phrase")."\">Expanded Text Search</a> which shows surrounding text.");
	}
}

modified: 

if ($phrase = $_REQUEST["phrase"])
{
	$phrase = preg_quote($this->htmlspecialchars_ent(stripslashes($phrase)), "/");
	$results = $this->FullTextSearch($phrase);
	$result_page_list = '';
	$total_results = 0;
	print("<br />");

	if ($results)
	{
		foreach ($results as $i => $page)
		{
			if ($this->HasAccess("read",$page["tag"]))
			{
				$result_page_list .= ($i+1).". ".$this->Link($page["tag"])."<br />\n";
				$total_results += 1;
			}
		}
		$phrase = urlencode($phrase);
	}

	$match_str = $total_results <> 1 ? " matches" : " match";
	print("Search results: <strong>".$total_results.$match_str."</strong> for <strong>$phrase</strong><br /><br />\n");
	if ($total_results > 0)
	{
		print($result_page_list);
		print("<br />Not sure which page to choose?<br />Try the <a href=\"".$this->href("", "TextSearchExpanded", "phrase=$phrase")."\">Expanded Text Search</a> which shows surrounding text.");
	}
}
  • Changes in actions/textsearchexpanded.php

original: 

if (isset($_REQUEST["phrase"]) && $phrase = $_REQUEST["phrase"])
{
	$phrase = stripslashes($phrase); 
	print("<br />");
	$results = $this->FullTextSearch($phrase);
	$match_str = count($results) <> 1 ? " matches" : " match";
	print("Search results: <strong>".count($results).$match_str."</strong> for <strong>$phrase</strong><br /><br />\n");
	$phrase = str_replace("\"", "", $phrase);
	  if ($results)
	  {
				print "<table border=\"0\" cellpadding=\"0\" cellspacing=\"0\">";
				$STORE_FORMATING_AS_TEXT = 1;
				foreach ($results as $i => $page)
				{
						//print(($i+1).". ".$this->Link($page["tag"])."<br />\n");
						//print implode($this->LoadPage($page["tag"]));
						//$matchString = preg_match("/(.{0,40}$phrase.{0,40})/",implode($this->LoadPage($page['tag'])));
						/* display portion of the matching body and highlight
						   the search term */
						preg_match("/(.{0,120}$phrase.{0,120})/is",$page['body'],$matchString);
						$text = $this->htmlspecialchars_ent($matchString[0]);
					 //   include("formatters/wakka.php");
						$highlightMatch = preg_replace("/($phrase)/i","<font color=\"green\"><b>$1</b></font>",$text,-1);
						$matchText = "<font color=\"gray\" size=\"-1\">...</font>$highlightMatch<font color=\"gray\" size=\"-1\">...</font>";
						print "

										  <tr>
												<td valign=\"top\" align=\"right\">
												  <!-- result number -->
												  <table>
														<tr>
														  <td valign=\"top\" align=\"left\" bgcolor=\"#DDDDDD\">
																<font color=\"white\" size=\"-3\">
																  ".($i+1)."
																</font>
														  </td>
														</tr>
												  </table>
												</td>
												<!-- link -->
												<td valign=\"top\">
												  ".$this->Link($page["tag"])."
												</td>
												<!-- date of last update -->
												<td valign=\"top\" align=\"right\">
												  <font color=\"gray\" size=\"-3\">
														$page[time]
												  </font>
												</td>
										  </tr>
										  <tr>
												<td>
												   
												</td>
												<td colspan=\"2\">
												  $matchText
												</td>
										  </tr>
										  <tr>
												<td>
												   
												</td>
										  </tr>

								";
				}
				print "</table>";
		}
}

modified: 

if (isset($_REQUEST["phrase"]) && $phrase = $_REQUEST["phrase"])
{
	$phrase = preg_quote($this->htmlspecialchars_ent(stripslashes($phrase)), "/");
	print("<br />");
	$results = $this->FullTextSearch($phrase);
	$phrase = str_replace("\"", "", $phrase);
	$result_page_list = '';
	$total_results = 0;

	if ($results)
	{
		print "<table border=\"0\" cellpadding=\"0\" cellspacing=\"0\">";
		$STORE_FORMATING_AS_TEXT = 1;
		foreach ($results as $i => $page)
		{
			if ($this->HasAccess("read",$page["tag"]))
			{
				preg_match("/(.{0,120}$phrase.{0,120})/is",$page['body'],$matchString);
				$text = $this->htmlspecialchars_ent($matchString[0]);
				$highlightMatch = preg_replace("/($phrase)/i","<font color=\"green\"><b>$1</b></font>",$text,-1);
				$matchText = "<font color=\"gray\" size=\"-1\">...</font>$highlightMatch<font color=\"gray\" size=\"-1\">...</font>";

				$total_results += 1;

				$result_page_list .= "
					<tr>
						<td valign=\"top\" align=\"right\">
							<!-- result number -->
							<table>
								<tr>
									<td valign=\"top\" align=\"left\" bgcolor=\"#DDDDDD\">
										<font color=\"white\" size=\"-3\">
											".($i+1)."
										</font>
									</td>
								</tr>
							</table>
						</td>
						<td valign=\"top\">
							<!-- link -->
							".$this->Link($page["tag"])."
						</td>
						<td valign=\"top\" align=\"right\">
							<!-- date of last update -->
							<font color=\"gray\" size=\"-3\">
								$page[time]
							</font>
						</td>
					</tr>
					<tr>
						<td> </td>
						<td colspan=\"2\">$matchText</td>
					</tr>
					<tr>
						<td> </td>
					</tr>
					";
			}

		}
		$match_str = $total_results <> 1 ? " matches" : " match";
		print("Search results: <strong>".$total_results.$match_str."</strong> for <strong>$phrase</strong><br /><br />\n");
		if ($total_results > 0)
		{
			print($result_page_list);
			print("</table>");
		}
	}
}

Attachments

textsearchexpanded.php Download (1.7 KB) - added by GiorgosKontopoulos 5 years ago.
the slightly modified expanded search that prevents sensitive info to be displayed
textsearch.php Download (2.1 KB) - added by GiorgosKontopoulos 5 years ago.
slightly modified TextSearch.php that handles international characters well

Change History

Changed 5 years ago by GiorgosKontopoulos

The revised code for TextSearchExpanded has 3 disadvantages 1)unacessary long 2)uses table for formating 3)has a bug (when the results are 0 the </table> is not send to the browser)

here is a slight modification of the original code that prevents unauthorized users from seeing confidential info

<code> <?php echo $this->FormOpen("", "", "get") ?> <table border="0" cellspacing="0" cellpadding="0">

<tr>

<td>Search for:&nbsp;</td> <td><input name="phrase" size="40" value="<?php if (isset($_REQUESTphrase?)) echo $this->htmlspecialchars_ent(stripslashes($_REQUESTphrase?)); ?>" /> <input type="submit" value="Search"/></td>

</tr>

</table><br /> <?php echo $this->FormClose(); ?>

<?php if (isset($_REQUESTphrase?) && $phrase = $_REQUESTphrase?) {

$phrase = preg_quote($this->htmlspecialchars_ent(stripslashes($phrase)), "/"); $results = $this->FullTextSearch($phrase); $phrase = str_replace("\"", "", $phrase); $phrase = preg_quote($phrase, "/"); if ($results) {

$resultsCount = 0; foreach ($results as $i => $page) {

if ($this->HasAccess("read",$pagetag?)) {

$resultsCount++; /* display portion of the matching body and highlight the search term */ preg_match("/(.{0,120}$phrase.{0,120})/is",$pagebody?,$matchString); $text = $this->htmlspecialchars_ent($matchString[0]); $highlightMatch = preg_replace("/($phrase)/i","<font color=\"green\"><b>$1</b></font>",$text,-1); $matchText = "&hellip;".$highlightMatch."&hellip;"; $output .= "\n<p>".($i+1)." ".$this->Link($pagetag?)." &mdash; ".$page[time]."</p>"; $output .= "\n<blockquote>".$matchText."</blockquote>\n";

}

} $match_str = $resultsCount <> 1 ? " matches" : " match"; print("Search results: <strong>".$resultsCount.$match_str."</strong> for <strong>".$this->htmlspecialchars_ent($phrase)."</strong><br />\n");

}

}

$output = $this->ReturnSafeHtml($output); echo $output; ?>

</code>

Changed 5 years ago by GiorgosKontopoulos

the slightly modified expanded search that prevents sensitive info to be displayed

Changed 5 years ago by GiorgosKontopoulos

slightly modified TextSearch.php that handles international characters well

Changed 4 years ago by DarTar

  • severity changed from normal to major

Thanks for the patches, Giorgos - listed for our next bugfix release.

Changed 4 years ago by DotMG

  • keywords Search Text Security Confidential added
  • owner changed from unassigned to DotMG

Other issues: 1) If page contains rêve and we search for reve, the page is listed but no snippet shown.

2) If page !XPageTitle does not contain this word !XPageTitle, and we search for !XPageTitle, the page is listed but no snippet shown

3) If page contains terms banana is better than apple and we the searchterm is +banana +apple, the page is listed but no snippet shown.

I did manage to fix these 3 issues with little changes.

Changed 4 years ago by DotMG

  • status changed from new to closed
  • resolution set to fixed
  • description modified (diff)

Fixed in [55].

Changed 4 years ago by DarTar

Mahefa,

whereas I find keyword highlighting useful, I'm not sure that displaying alternate colours for snippets is a good idea. I don't have the feeling this increases readability and I'd prefer to stick to a simple blockquote (alternate colouring make sense IMO only in tables). What do others think of this?

Changed 4 years ago by NilsLindenberg

You speake about the green color used to highlight the search term? I find it a nice idea. Perhaps we should just wait and see how many people complain ;)

Changed 4 years ago by NilsLindenberg

Ah, now I see. Perhaps better to have a color for the "header" (page-name) etc. and one for the snippets? Otherwise see the last sentence of my last comment.

Changed 4 years ago by DarTar

Yes, I'm referring to the alternate row styling, not to the keyword highlighting idea which I like. Here's my main concerns:

  • alternate styling usually applies to tables, where it is supposed to help (horizontal) row readability. It's quite strange (and I 'm not familiar with other cases) to see alternate styling for codeblocks and without a tabular structure.
  • if we agree alternate styling is a good option for text search results, then IMO it should 1. become configurable and 2. use discrete shadings.
Note: See TracTickets for help on using tickets.