Fix potential SQL injection in SelectLimit()

The `SelectLimit` function has a potential SQL injection vulnerability through the use of the `nrows` and `offset` parameters which are not forced to integers. This is a follow-up on ADOdb#311, and fixes all remaining drivers that do not use ADOConnection::SelectLimit(). Fixes ADOdb#401 Signed-off-by: Damien Regad <dregad@mantisbt.org> Original commits squashed, message reworded. Fixed whitespace.
dregad · Mar 30, 2018 · d29c23f · d29c23f
1 parent 34788ce
commit d29c23f
Show file tree

Hide file tree

Showing 13 changed files with 37 additions and 12 deletions.
diff --git a/drivers/adodb-borland_ibase.inc.php b/drivers/adodb-borland_ibase.inc.php
@@ -53,6 +53,8 @@ function ServerInfo()
 	//		SELECT FIRST 5 SKIP 2 col1, col2 FROM TABLE
 	function SelectLimit($sql,$nrows=-1,$offset=-1,$inputarr=false,$secs2cache=0)
 	{
+		$nrows = (int) $nrows;
+		$offset = (int) $offset;
 		if ($nrows > 0) {
 			if ($offset <= 0) $str = " ROWS $nrows ";
 			else {

diff --git a/drivers/adodb-csv.inc.php b/drivers/adodb-csv.inc.php
@@ -83,8 +83,10 @@ function MetaColumns($table, $normalize=true)
 	// parameters use PostgreSQL convention, not MySQL
 	function SelectLimit($sql, $nrows = -1, $offset = -1, $inputarr = false, $secs2cache = 0)
 	{
-	global $ADODB_FETCH_MODE;
+		global $ADODB_FETCH_MODE;
 
+		$nrows = (int) $nrows;
+		$offset = (int) $offset;
 		$url = $this->_url.'?sql='.urlencode($sql)."&nrows=$nrows&fetch=".
 			(($this->fetchMode !== false)?$this->fetchMode : $ADODB_FETCH_MODE).
 			"&offset=$offset";

diff --git a/drivers/adodb-db2.inc.php b/drivers/adodb-db2.inc.php
@@ -234,7 +234,8 @@ function DropSequence($seqname = 'adodbseq')
 
 	function SelectLimit($sql, $nrows = -1, $offset = -1, $inputArr = false, $secs2cache = 0)
 	{
-		$nrows = (integer) $nrows;
+		$nrows = (int) $nrows;
+		$offset = (int) $offset;
 		if ($offset <= 0) {
 		// could also use " OPTIMIZE FOR $nrows ROWS "
 			if ($nrows >= 0) $sql .=  " FETCH FIRST $nrows ROWS ONLY ";

diff --git a/drivers/adodb-mssql.inc.php b/drivers/adodb-mssql.inc.php
@@ -245,6 +245,8 @@ function GenID($seq='adodbseq',$start=1)
 
 	function SelectLimit($sql,$nrows=-1,$offset=-1, $inputarr=false,$secs2cache=0)
 	{
+		$nrows = (int) $nrows;
+		$offset = (int) $offset;
 		if ($nrows > 0 && $offset <= 0) {
 			$sql = preg_replace(
 				'/(^\s*select\s+(distinctrow|distinct)?)/i','\\1 '.$this->hasTop." $nrows ",$sql);

diff --git a/drivers/adodb-mysql.inc.php b/drivers/adodb-mysql.inc.php
@@ -585,6 +585,8 @@ function SelectDB($dbName)
 	// parameters use PostgreSQL convention, not MySQL
 	function SelectLimit($sql,$nrows=-1,$offset=-1,$inputarr=false,$secs=0)
 	{
+		$nrows = (int) $nrows;
+		$offset = (int) $offset;
 		$offsetStr =($offset>=0) ? ((integer)$offset)."," : '';
 		// jason judge, see http://phplens.com/lens/lensforum/msgs.php?id=9220
 		if ($nrows < 0) $nrows = '18446744073709551615';

diff --git a/drivers/adodb-mysqli.inc.php b/drivers/adodb-mysqli.inc.php
@@ -713,6 +713,8 @@ function SelectLimit($sql,
 				$inputarr = false,
 				$secs = 0)
 	{
+		$nrows = (int) $nrows;
+		$offset = (int) $offset;
 		$offsetStr = ($offset >= 0) ? "$offset," : '';
 		if ($nrows < 0) $nrows = '18446744073709551615';
 

diff --git a/drivers/adodb-oci8.inc.php b/drivers/adodb-oci8.inc.php
@@ -709,6 +709,8 @@ function GetRandRow($sql, $arr = false)
 	 */
 	function SelectLimit($sql,$nrows=-1,$offset=-1, $inputarr=false,$secs2cache=0)
 	{
+		$nrows = (int) $nrows;
+		$offset = (int) $offset;
 		// Since the methods used to limit the number of returned rows rely
 		// on modifying the provided SQL query, we can't work with prepared
 		// statements so we just extract the SQL string.

diff --git a/drivers/adodb-odbc_mssql.inc.php b/drivers/adodb-odbc_mssql.inc.php
@@ -280,6 +280,8 @@ function MetaPrimaryKeys($table, $owner = false)
 
 	function SelectLimit($sql,$nrows=-1,$offset=-1, $inputarr=false,$secs2cache=0)
 	{
+		$nrows = (int) $nrows;
+		$offset = (int) $offset;
 		if ($nrows > 0 && $offset <= 0) {
 			$sql = preg_replace(
 				'/(^\s*select\s+(distinctrow|distinct)?)/i','\\1 '.$this->hasTop." $nrows ",$sql);

diff --git a/drivers/adodb-pdo_pgsql.inc.php b/drivers/adodb-pdo_pgsql.inc.php
@@ -73,12 +73,14 @@ function ServerInfo()
 
 	function SelectLimit($sql,$nrows=-1,$offset=-1,$inputarr=false,$secs2cache=0)
 	{
-		 $offsetStr = ($offset >= 0) ? " OFFSET $offset" : '';
-		 $limitStr  = ($nrows >= 0)  ? " LIMIT $nrows" : '';
-		 if ($secs2cache)
-		  	$rs = $this->CacheExecute($secs2cache,$sql."$limitStr$offsetStr",$inputarr);
-		 else
-		  	$rs = $this->Execute($sql."$limitStr$offsetStr",$inputarr);
+		$nrows = (int) $nrows;
+		$offset = (int) $offset;
+		$offsetStr = ($offset >= 0) ? " OFFSET $offset" : '';
+		$limitStr  = ($nrows >= 0)  ? " LIMIT $nrows" : '';
+		if ($secs2cache)
+			$rs = $this->CacheExecute($secs2cache,$sql."$limitStr$offsetStr",$inputarr);
+		else
+			$rs = $this->Execute($sql."$limitStr$offsetStr",$inputarr);
 
 		return $rs;
 	}

diff --git a/drivers/adodb-pdo_sqlite.inc.php b/drivers/adodb-pdo_sqlite.inc.php
@@ -54,13 +54,15 @@ function ServerInfo()
 
 	function SelectLimit($sql,$nrows=-1,$offset=-1,$inputarr=false,$secs2cache=0)
 	{
+		$nrows = (int) $nrows;
+		$offset = (int) $offset;
 		$parent = $this->pdoDriver;
 		$offsetStr = ($offset >= 0) ? " OFFSET $offset" : '';
 		$limitStr  = ($nrows >= 0)  ? " LIMIT $nrows" : ($offset >= 0 ? ' LIMIT 999999999' : '');
-	  	if ($secs2cache)
-	   		$rs = $parent->CacheExecute($secs2cache,$sql."$limitStr$offsetStr",$inputarr);
-	  	else
-	   		$rs = $parent->Execute($sql."$limitStr$offsetStr",$inputarr);
+		if ($secs2cache)
+			$rs = $parent->CacheExecute($secs2cache,$sql."$limitStr$offsetStr",$inputarr);
+		else
+			$rs = $parent->Execute($sql."$limitStr$offsetStr",$inputarr);
 
 		return $rs;
 	}

diff --git a/drivers/adodb-postgres7.inc.php b/drivers/adodb-postgres7.inc.php
@@ -109,6 +109,8 @@ function __construct()
 	// which makes obsolete the LIMIT limit,offset syntax
 	function SelectLimit($sql,$nrows=-1,$offset=-1,$inputarr=false,$secs2cache=0)
 	{
+		$nrows = (int) $nrows;
+		$offset = (int) $offset;
 		$offsetStr = ($offset >= 0) ? " OFFSET ".((integer)$offset) : '';
 		$limitStr  = ($nrows >= 0)  ? " LIMIT ".((integer)$nrows) : '';
 		if ($secs2cache)

diff --git a/drivers/adodb-sqlite.inc.php b/drivers/adodb-sqlite.inc.php
@@ -226,6 +226,8 @@ function _query($sql,$inputarr=false)
 
 	function SelectLimit($sql,$nrows=-1,$offset=-1,$inputarr=false,$secs2cache=0)
 	{
+		$nrows = (int) $nrows;
+		$offset = (int) $offset;
 		$offsetStr = ($offset >= 0) ? " OFFSET $offset" : '';
 		$limitStr  = ($nrows >= 0)  ? " LIMIT $nrows" : ($offset >= 0 ? ' LIMIT 999999999' : '');
 		if ($secs2cache) {

diff --git a/drivers/adodb-sqlite3.inc.php b/drivers/adodb-sqlite3.inc.php
@@ -209,6 +209,8 @@ function _query($sql,$inputarr=false)
 
 	function SelectLimit($sql,$nrows=-1,$offset=-1,$inputarr=false,$secs2cache=0)
 	{
+		$nrows = (int) $nrows;
+		$offset = (int) $offset;
 		$offsetStr = ($offset >= 0) ? " OFFSET $offset" : '';
 		$limitStr  = ($nrows >= 0)  ? " LIMIT $nrows" : ($offset >= 0 ? ' LIMIT 999999999' : '');
 		if ($secs2cache) {