revisited sql escaping

in general, its not nessecary to manually apply escapes
- if passed as param to DB::DBType()->queryFunc() its escaped by DBsimple
- if passed as condition its escaped by BaseType
- if you do your own query use DB::DBType()->escape() on strings and intVal() respectively

- fixed use of HAVING in CreatureListFilter
- allow for exclude-only searches in Filter
- BaseType no longer adds %-wildcards to strings. Do this manually if ever nessecary.

includes/types/basetype.class.php

@@ -21,7 +21,7 @@ abstract class BaseType
     *       expression:    str   - must match fieldname;
     *                      int   - 1: select everything; 0: select nothing
     *                      array - another condition array
-    *       value:         str   - operator defaults to: LIKE %<val>%
+    *       value:         str   - operator defaults to: LIKE <val>
     *                      int   - operator defaults to: = <val>
     *                      array - operator defaults to: IN (<val>)
     *                      null  - operator defaults to: IS [NULL]
@@ -35,7 +35,7 @@ abstract class BaseType
     *   example:
     *       array(
     *           ['id', 45],
-    *           ['name', 'test', '!'],
+    *           ['name', 'test%', '!'],
     *           [
     *               'AND',
     *               ['flags', 0xFF, '&'],
@@ -47,9 +47,9 @@ abstract class BaseType
     *           5
     *       )
     *   results in
-    *       WHERE ((`id` = 45) OR (`name` NOT LIKE "%test%") OR ((`flags` & 255) AND (`flags2` & 15)) OR ((`mask` & 3) = 0)) OR (`joinedTbl`.`field` IS NULL) LIMIT 5
+    *       WHERE ((`id` = 45) OR (`name` NOT LIKE "test%") OR ((`flags` & 255) AND (`flags2` & 15)) OR ((`mask` & 3) = 0)) OR (`joinedTbl`.`field` IS NULL) LIMIT 5
     */
-    public function __construct($conditions = [])
+    public function __construct($conditions = [], $miscData = null)
     {
         $where     = [];
         $linking   = ' AND ';
@@ -65,6 +65,9 @@ abstract class BaseType
         else
             $prefixes['base'] = '';
 
+        if ($miscData && !empty($miscData['extraOpts']))
+            $this->extendQueryOpts($miscData['extraOpts']);
+
         $resolveCondition = function ($c, $supLink) use (&$resolveCondition, &$prefixes)
         {
             $subLink = '';
@@ -107,17 +110,22 @@ abstract class BaseType
                         if (is_array($f))
                             $f = $f[0];
 
-                        if (is_numeric($f))
+                        // numeric allows for formulas e.g. (1 < 3)
+                        if (Util::checkNumeric($f))
                             return $f;
 
-                        $f = explode('.', Util::sqlEscape($f));
+                        // skip condition if fieldName contains illegal chars
+                        if (preg_match('/[^\d\w\.\_]/i', $f))
+                            return null;
+
+                        $f = explode('.', $f);
 
                         switch (count($f))
                         {
                             case 2:
                                 if (!in_array($f[0], $prefixes))
                                 {
-                                    // choose table to join or return null if prefix not existant
+                                    // choose table to join or return null if prefix does not exist
                                     if (!in_array($f[0], array_keys($this->queryOpts)))
                                         return null;
 
@@ -146,12 +154,12 @@ abstract class BaseType
 
                 if (is_array($c[1]))
                 {
-                    $val = implode(',', Util::sqlEscape($c[1], true));
+                    array_walk($c[1], function(&$item, $key) {
-                    if ($val === '')
+                         $item = Util::checkNumeric($item) ? $item : DB::Aowow()->escape($item);
-                        return null;
+                    });
 
                     $op  = (isset($c[2]) && $c[2] == '!') ? 'NOT IN' : 'IN';
-                    $val = '('.$val.')';
+                    $val = '('.implode(', ', $c[1]).')';
                 }
                 else if (Util::checkNumeric($c[1]))
                 {
@@ -160,18 +168,8 @@ abstract class BaseType
                 }
                 else if (is_string($c[1]))
                 {
-                    $val = Util::sqlEscape($c[1], true);
-
-                    /*
-                        long term observation
-                        do both methods diff ?
-                    */
-                    $debug = DB::Aowow()->escape($c[1]);
-                    if ($debug != "'".$val."'")
-                        Util::$pageTemplate->internalNotice(U_GROUP_ADMIN, 'BaseType::__construct() - escape mechanism have different results: \''.$val.'\' => '.$debug);
-
                     $op  = (isset($c[2]) && $c[2] == '!') ? 'NOT LIKE' : 'LIKE';
-                    $val = $val === '' ? '""' : '"%'.$val.'%"';
+                    $val = DB::Aowow()->escape($c[1]);
                 }
                 else if (count($c) > 1 && $c[1] === null)   // specifficly check for NULL
                 {
@@ -761,7 +759,6 @@ abstract class Filter
             $string = $this->fiData['v']['na'];
 
         $qry   = [];
-        $valid = false;
         foreach ($fields as $n => $f)
         {
             $sub = [];
@@ -770,12 +767,9 @@ abstract class Filter
             foreach ($parts as $p)
             {
                 if ($p[0] == '-' && strlen($p) > 3)
-                    $sub[] = [$f, substr($p, 1), '!'];
+                    $sub[] = [$f, '%'.substr($p, 1).'%', '!'];
                 else if ($p[0] != '-' && strlen($p) > 2)
-                {
+                    $sub[] = [$f, '%'.$p.'%'];
-                    $valid = true;
-                    $sub[] = [$f, $p];
-                }
             }
 
             // single cnd?
@@ -789,14 +783,10 @@ abstract class Filter
             $qry[] = $sub;
         }
 
-        if (!$valid)                                        // no +term with length >= 3 set
-        {
-            $this->error = 1;
-            return [];
-        }
-
         // single cnd?
-        if (count($qry) > 1)
+        if (!$qry)
+            $this->error = 1;
+        else if (count($qry) > 1)
             array_unshift($qry, 'OR');
         else
             $qry = $qry[0];
@@ -949,7 +939,6 @@ abstract class Filter
         return $result;
     }
 
-    // apply Util::sqlEscape() and intVal() in the implementation of these
     abstract protected function createSQLForCriterium(&$cr);
     abstract protected function createSQLForValues();
 }

includes/types/creature.class.php

@@ -228,6 +228,7 @@ class CreatureList extends BaseType
 
 class CreatureListFilter extends Filter
 {
+    public    $extraOpts     = null;
     protected $enums         = array(
         3 => array( 469, 1037, 1106,  529, 1012,   87,   21,  910,  609,  942,  909,  530,   69,  577,  930, 1068, 1104,  729,  369,   92,   54,  946,   67, 1052,  749,
                      47,  989, 1090, 1098,  978, 1011,   93, 1015, 1038,   76,  470,  349, 1031, 1077,  809,  911,  890,  970,  169,  730,   72,   70,  932, 1156,  933,
@@ -279,20 +280,20 @@ class CreatureListFilter extends Filter
                 switch ($cr[1])
                 {
                     case '=':                               // min > max is totally possible
-                        $this->parent->queryOpts['clsMin']['h'] = 'IF(healthMin > healthMax, healthMax, healthMin) <= '.$cr[2];
+                        $this->extraOpts['clsMin']['h'] = 'IF(healthMin > healthMax, healthMax, healthMin) <= '.$cr[2];
-                        $this->parent->queryOpts['clsMax']['h'] = 'IF(healthMin > healthMax, healthMin, healthMax) >= '.$cr[2];
+                        $this->extraOpts['clsMax']['h'] = 'IF(healthMin > healthMax, healthMin, healthMax) >= '.$cr[2];
                         break;
                     case '>':
-                        $this->parent->queryOpts['clsMin']['h'] = 'IF(healthMin > healthMax, healthMax, healthMin) > '.$cr[2];
+                        $this->extraOpts['clsMin']['h'] = 'IF(healthMin > healthMax, healthMax, healthMin) > '.$cr[2];
                         break;
                     case '>=':
-                        $this->parent->queryOpts['clsMin']['h'] = 'IF(healthMin > healthMax, healthMax, healthMin) >= '.$cr[2];
+                        $this->extraOpts['clsMin']['h'] = 'IF(healthMin > healthMax, healthMax, healthMin) >= '.$cr[2];
                         break;
                     case '<':
-                        $this->parent->queryOpts['clsMax']['h'] = 'IF(healthMin > healthMax, healthMin, healthMax) < '.$cr[2];
+                        $this->extraOpts['clsMax']['h'] = 'IF(healthMin > healthMax, healthMin, healthMax) < '.$cr[2];
                         break;
                     case '<=':
-                        $this->parent->queryOpts['clsMax']['h'] = 'IF(healthMin > healthMax, healthMin, healthMax) <= '.$cr[2];
+                        $this->extraOpts['clsMax']['h'] = 'IF(healthMin > healthMax, healthMin, healthMax) <= '.$cr[2];
                         break;
                 }
                 return [1];                                 // always true, use post-filter
@@ -304,20 +305,20 @@ class CreatureListFilter extends Filter
                 switch ($cr[1])
                 {
                     case '=':
-                        $this->parent->queryOpts['clsMin']['h'] = 'IF(manaMin > manaMax, manaMax, manaMin) <= '.$cr[2];
+                        $this->extraOpts['clsMin']['h'] = 'IF(manaMin > manaMax, manaMax, manaMin) <= '.$cr[2];
-                        $this->parent->queryOpts['clsMax']['h'] = 'IF(manaMin > manaMax, manaMin, manaMax) => '.$cr[2];
+                        $this->extraOpts['clsMax']['h'] = 'IF(manaMin > manaMax, manaMin, manaMax) => '.$cr[2];
                         break;
                     case '>':
-                        $this->parent->queryOpts['clsMax']['h'] = 'IF(manaMin > manaMax, manaMin, manaMax) > '.$cr[2];
+                        $this->extraOpts['clsMax']['h'] = 'IF(manaMin > manaMax, manaMin, manaMax) > '.$cr[2];
                         break;
                     case '>=':
-                        $this->parent->queryOpts['clsMax']['h'] = 'IF(manaMin > manaMax, manaMin, manaMax) >= '.$cr[2];
+                        $this->extraOpts['clsMax']['h'] = 'IF(manaMin > manaMax, manaMin, manaMax) >= '.$cr[2];
                         break;
                     case '<':
-                        $this->parent->queryOpts['clsMin']['h'] = 'IF(manaMin > manaMax, manaMax, manaMin) < '.$cr[2];
+                        $this->extraOpts['clsMin']['h'] = 'IF(manaMin > manaMax, manaMax, manaMin) < '.$cr[2];
                         break;
                     case '<=':
-                        $this->parent->queryOpts['clsMin']['h'] = 'IF(manaMin > manaMax, manaMax, manaMin) <= '.$cr[2];
+                        $this->extraOpts['clsMin']['h'] = 'IF(manaMin > manaMax, manaMax, manaMin) <= '.$cr[2];
                         break;
                 }
                 return [1];                                 // always true, use post-filter

includes/types/item.class.php

@@ -30,11 +30,7 @@ class ItemList extends BaseType
 
     public function __construct($conditions = [], $miscData = null)
     {
-        // search by statweight
+        parent::__construct($conditions, $miscData);
-        if ($miscData && !empty($miscData['extraOpts']))
-            $this->extendQueryOpts($miscData['extraOpts']);
-
-        parent::__construct($conditions);
 
         foreach ($this->iterate() as &$_curTpl)
         {
@@ -1831,7 +1827,7 @@ class ItemListFilter extends Filter
                                 reagentCount1, reagentCount2, reagentCount3, reagentCount4, reagentCount5, reagentCount6, reagentCount7, reagentCount8
                         FROM    ?_spell
                         WHERE   skillLine1 IN (?a)',
-                        is_bool($_) ? array_filter($this->enums[$cr[0]], "is_numeric") : $_
+                        is_bool($_) ? array_filter($this->enums[99], "is_numeric") : $_
                     );
                     foreach ($spells as $spell)
                         for ($i = 1; $i < 9; $i++)

includes/utilities.php

@@ -1253,21 +1253,6 @@ class Util
         return 'b'.strToUpper($_);
     }
 
-    public static function sqlEscape($data, $relaxed = false)
-    {
-        // relaxed: expecting strings for fulltext search
-        $pattern = $relaxed ? ['/[;`´"\/\\\]/ui', '--'] : ['/[^\p{L}0-9\s_\-\.]/ui', '--'];
-
-        if (!is_array($data))
-            return preg_replace($pattern, '', trim($data));
-
-        array_walk($data, function(&$item, $key) use (&$relaxed) {
-            $item = self::sqlEscape($item, $relaxed);
-        });
-
-        return $data;
-    }
-
     public static function jsEscape($string)
     {
         return strtr(trim($string), array(
@@ -2079,7 +2064,6 @@ class Util
                     }
 
                     /*
-
                         todo: search for achievements here
                         $tabsFinal[17]
                     */
@@ -2132,7 +2116,7 @@ class Util
                             else if ($tabId < 0 && $curTpl['typeFlags'] & NPC_TYPEFLAG_MININGLOOT)
                                 $tabId = 7;
                             else if ($tabId < 0)
-                                $tabId = abs($tabId);               // general case (skinning)
+                                $tabId = abs($tabId);       // general case (skinning)
 
                             $tabsFinal[$tabId][1][] = array_merge($srcData[$srcObj->id], $result[$srcObj->getField($field)]);
                             $tabsFinal[$tabId][4][] = 'Listview.extraCols.percent';

pages/account.php

@@ -59,10 +59,7 @@ function signin()
             $_SERVER['REMOTE_ADDR']
         );
 
-    $id = DB::Aowow()->SelectCell('SELECT id FROM ?_account WHERE user = ?',
+    $id = DB::Aowow()->SelectCell('SELECT id FROM ?_account WHERE user = ?', $username);
-        Util::sqlEscape($username)
-    );
-
     if (!$id)
         return Lang::$account['userNotFound'];
 
@@ -273,8 +270,6 @@ if (User::$id)
             $next = !empty($next[1]) ? '?'.$next[1] : '.';
             header('Location: '.$next);
         case 'weightscales':
-            $post = Util::sqlEscape($_POST, true);
-
             if (isset($post['save']))
             {
                 if (!isset($post['id']))
@@ -292,7 +287,7 @@ if (User::$id)
                     die('0');
             }
             else if (isset($post['delete']) && isset($post['id']))
-                DB::Aowow()->query('DELETE FROM ?_account_weightscales WHERE id = ?d AND account = ?d', $post['id'], User::$id);
+                DB::Aowow()->query('DELETE FROM ?_account_weightscales WHERE id = ?d AND account = ?d', intVal($post['id']), User::$id);
             else
                 die('0');
 

pages/item.php

@@ -80,15 +80,7 @@ if (isset($_GET['xml']))
     if (!$smarty->loadCache($cacheKeyXML, $root))
     {
         $root = new SimpleXML('<aowow />');
-
+        $cnd  = array($_id ? ['i.id', $_id] : ['name_loc'.User::$localeId, $pageParam]);
-        if (!$_id)
-        {
-            $str = DB::Aowow()->escape(urlDecode($pageParam));
-            $str = substr($str, 1, -1);                     // escape adds '
-            $cnd = array(['name_loc'.User::$localeId, $str]);
-        }
-        else
-            $cnd = array(['i.id', $_id]);
 
         $item = new ItemList($cnd);
         if ($item->error)

pages/npcs.php

@@ -29,7 +29,8 @@ if (!$smarty->loadCache($cacheKey, $pageData, $filter))
     if ($_ = $npcFilter->getConditions())
         $conditions[] = $_;
 
-    $npcs = new CreatureList($conditions);                  // beast subtypes are selected via filter
+    // beast subtypes are selected via filter
+    $npcs = new CreatureList($conditions, ['extraOpts' => $npcFilter->extraOpts]);
 
     // recreate form selection
     $filter = array_merge($npcFilter->getForm('form'), $filter);

search.php

@@ -38,7 +38,7 @@ $_wtv       = isset($_GET['wtv']) ? explode(':', $_GET['wtv']) : null;
 $_slots     = [];
 
 $search     = urlDecode(trim($pageParam));
-$query      = Util::sqlEscape(str_replace('?', '_', str_replace('*', '%', ($search))), true);
+$query      = strtr($search, '?*', '_%');
 $invalid    = [];
 $include    = [];
 $exclude    = [];
@@ -73,10 +73,10 @@ $createLookup = function(array $fields = []) use($include, $exclude)
     {
         $sub = [];
         foreach ($include as $i)
-            $sub[] = [$f, $i];
+            $sub[] = [$f, '%'.$i.'%'];
 
         foreach ($exclude as $x)
-            $sub[] = [$f, $x, '!'];
+            $sub[] = [$f, '%'.$x.'%', '!'];
 
         // single cnd?
         if (count($sub) > 1)
@@ -152,12 +152,12 @@ if ((!$include || !($searchMask & SEARCH_MASK_ALL)) && !($searchMask & SEARCH_TY
     else if ($searchMask & SEARCH_TYPE_OPEN)
     {
         header("Content-type: text/javascript");
-        exit('["'.Util::jsEscape($query).'", []]');
+        exit('["'.Util::jsEscape($search).'", []]');
     }
     else if (!$_wt || !$_wtv)                               // implicitly: SEARCH_TYPE_JSON
     {
         header("Content-type: text/javascript");
-        exit ("[\"".Util::jsEscape($query)."\", [\n],[\n]]\n");
+        exit ("[\"".Util::jsEscape($search)."\", [\n],[\n]]\n");
     }
 }
 
@@ -1088,7 +1088,7 @@ if ($searchMask & SEARCH_TYPE_JSON)
     }
 
     header("Content-type: text/javascript");
-    die ('["'.Util::jsEscape($query)."\", [\n".$outItems."],[\n".$outSets.']]');
+    die ('["'.Util::jsEscape($search)."\", [\n".$outItems."],[\n".$outSets.']]');
 }
 else if ($searchMask & SEARCH_TYPE_OPEN)
 {
@@ -1102,7 +1102,7 @@ else if ($searchMask & SEARCH_TYPE_OPEN)
         $foundTotal += $tmp['matches'];
 
     if (!$foundTotal)
-        exit('["'.Util::jsEscape($query).'", []]');
+        exit('["'.Util::jsEscape($search).'", []]');
 
     foreach ($found as $id => $set)
     {