[Solved] Totally getting rid of SQL injections

[prip]

Hi there,

While prestashop is in the beginning of it is life, to make this software more secure and getting rid of sql injection attempts until forever you may make some minor changes in the sql functins and query types.

As you did, prestashop has already a mysql class and a query function in it, so you can make queries more secure;

In MySQL.php

public function Execute($query,$variables)
{
if (parent::blacklist($query))
return false;
$this->_result = false;

$i=1;
foreach($variables as $variable){
$query = str_replace('{'.$i.'}',mysql_real_escape_string($variable),$query);
$i++;
}

if ($this->_link)
{
$this->_result = mysql_query($query, $this->_link);
return $this->_result;
}
return false;
}

And for queries;

$sql->execute("SELECT falan,filan FROM falanca WHERE id = '{1}' AND name = '{2}' ",array($_POST["id"],$_POST["name"]));

and of course for insert,delete,update too. Also it is more proper to make typecasting rather than treat all inputs as a string.

[hydra]

Hi,

I think that there should be a reply to this from a Presta developer.
I am not that good with programming but always interested in these security enhancements.

Ronald.

[Matthieu Biart]

Hi prip & hydra!

I do not think that suggestion is so usefull in our case. No offence but knowing how variables are controlled and casted before going into the method Execute, it does not need such treatment.
Before any calling to Execute, datas are casted (with intval() or floatval()) or backslashed by the function pSQL which in a way is quite similar to your suggestion.

And I think putting variables directly from post into a query is kind of dirty but it is just my opinion.

[prip]

In my opinion, it is useless to write pSQL or intval or floatval or any other filtering function for each variable in each query by hand. Also you may forget to write escapin functions or module writers any other than your team may not be careful as you. The proper solution is making type casting and escaping easier and more logical like this;

 $sql->execute(“SELECT falan,filan FROM falanca WHERE id = ‘{1}’ AND name = ‘{2}’ AND price = '{3}' “,array(array($_POST[“id”],1),array($_POST[“name”],0),array($_POST["price"],2)));

$i=1;
foreach($variables as $variablesAndTypes){ 
 switch($variablesAndTypes[1]){
    case 1:
      $query = str_replace(’{’.$i.’}’,intval($variablesAndTypes[0])),$query); 
    break;
    case 2:
      $query = str_replace(’{’.$i.’}’,floatval($variablesAndTypes[0])),$query); 
    break;
    default:
      $query = str_replace(’{’.$i.’}’,mysql_real_escape_string($variablesAndTypes[0])),$query); 
    break;
 }
$i++; 
}

[Michael]

i"m dum where to put this script in mysql.php???? at the end???????????

[prip]

do not put the codes are only for demonstration it will not work and break things also.