From cb68c863f06861892f6b55f8c273c928feb9cc44 Mon Sep 17 00:00:00 2001
From: Tim Starling <tstarling@users.mediawiki.org>
Date: Mon, 22 Dec 2008 12:31:15 +0000
Subject: [PATCH] Make the SQL search subclasses less nerve-wracking to read,
 by using makeList() instead of implode(), addQuotes() instead of strencode(),
 and by documenting the fact that parseQuery() is intentionally returning an
 SQL fragment to be included into a query without further escaping. No actual
 vulnerabilities fixed, due to effective UI-side validation, so this is just
 to minimise reviewer anxiety.

---
 includes/SearchMySQL.php    | 10 +++++++---
 includes/SearchOracle.php   | 14 +++++++++-----
 includes/SearchPostgres.php |  8 +++++---
 3 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/includes/SearchMySQL.php b/includes/SearchMySQL.php
index 984fcd892a..5fc06790f3 100644
--- a/includes/SearchMySQL.php
+++ b/includes/SearchMySQL.php
@@ -34,7 +34,10 @@ class SearchMySQL extends SearchEngine {
 		$this->db = $db;
 	}
 
-	/** @todo document */
+	/** 
+	 * Parse the user's query and transform it into an SQL fragment which will 
+	 * become part of a WHERE clause
+	 */
 	function parseQuery( $filteredText, $fulltext ) {
 		global $wgContLang;
 		$lc = SearchEngine::legalSearchChars(); // Minus format chars
@@ -126,9 +129,10 @@ class SearchMySQL extends SearchEngine {
 	function queryNamespaces() {
 		if( is_null($this->namespaces) )
 			return '';  # search all
-		$namespaces = implode( ',', $this->namespaces );
-		if ($namespaces == '') {
+		if ( !count( $this->namespaces ) ) {
 			$namespaces = '0';
+		} else {
+			$namespaces = $this->db->makeList( $this->namespaces );
 		}
 		return 'AND page_namespace IN (' . $namespaces . ')';
 	}
diff --git a/includes/SearchOracle.php b/includes/SearchOracle.php
index bf9368d150..b48d5e6e41 100644
--- a/includes/SearchOracle.php
+++ b/includes/SearchOracle.php
@@ -77,9 +77,10 @@ class SearchOracle extends SearchEngine {
 	function queryNamespaces() {
 		if( is_null($this->namespaces) )
 			return '';
-		$namespaces = implode(',', $this->namespaces);
-		if ($namespaces == '') {
+		if ( !count( $this->namespaces ) ) {
 			$namespaces = '0';
+		} else {
+			$namespaces = $this->db->makeList( $this->namespaces );
 		}
 		return 'AND page_namespace IN (' . $namespaces . ')';
 	}
@@ -144,7 +145,10 @@ class SearchOracle extends SearchEngine {
 			'WHERE page_id=si_page AND ' . $match;
 	}
 
-	/** @todo document */
+	/** 
+	 * Parse a user input search string, and return an SQL fragment to be used 
+	 * as part of a WHERE clause
+	 */
 	function parseQuery($filteredText, $fulltext) {
 		global $wgContLang;
 		$lc = SearchEngine::legalSearchChars();
@@ -170,9 +174,9 @@ class SearchOracle extends SearchEngine {
 			}
 		}
 
-		$searchon = $this->db->strencode(join(',', $q));
+		$searchon = $this->db->addQuotes(join(',', $q));
 		$field = $this->getIndexField($fulltext);
-		return " CONTAINS($field, '$searchon', 1) > 0 ";
+		return " CONTAINS($field, $searchon, 1) > 0 ";
 	}
 
 	/**
diff --git a/includes/SearchPostgres.php b/includes/SearchPostgres.php
index 7f15f67204..4862a44e57 100644
--- a/includes/SearchPostgres.php
+++ b/includes/SearchPostgres.php
@@ -66,6 +66,7 @@ class SearchPostgres extends SearchEngine {
 
 	/*
 	 * Transform the user's search string into a better form for tsearch2
+	 * Returns an SQL fragment consisting of quoted text to search for.
 	*/
 	function parseQuery( $term ) {
 
@@ -142,6 +143,7 @@ class SearchPostgres extends SearchEngine {
 		}
 		$prefix = $wgDBversion < 8.3 ? "'default'," : '';
 
+		# Get the SQL fragment for the given term
 		$searchstring = $this->parseQuery( $term );
 
 		## We need a separate query here so gin does not complain about empty searches
@@ -183,7 +185,7 @@ class SearchPostgres extends SearchEngine {
 			if ( count($this->namespaces) < 1)
 				$query .= ' AND page_namespace = 0';
 			else {
-				$namespaces = implode( ',', $this->namespaces );
+				$namespaces = $this->db->makeList( $this->namespaces );
 				$query .= " AND page_namespace IN ($namespaces)";
 			}
 		}
@@ -202,8 +204,8 @@ class SearchPostgres extends SearchEngine {
 	function update( $pageid, $title, $text ) {
 		## We don't want to index older revisions
 		$SQL = "UPDATE pagecontent SET textvector = NULL WHERE old_id IN ".
-				"(SELECT rev_text_id FROM revision WHERE rev_page = $pageid ".
-				"ORDER BY rev_text_id DESC OFFSET 1)";
+				"(SELECT rev_text_id FROM revision WHERE rev_page = " . intval( $pageid ) . 
+				" ORDER BY rev_text_id DESC OFFSET 1)";
 		$this->db->doQuery($SQL);
 		return true;
 	}
-- 
2.20.1