Is there a good class in PHP which allows easy use of mysql use and at the same time makes it secure for SQL Injection ETC.

Does PEAR implement most security measures?

I've read lots of this forum but everyone seems to have different ideas to prevent SQL injection in PHP.

I guess there are a few classes, but mainly what I use are custom uilt functions. PHPBB3.0 has a very nice SQL PHP class embeded into their new software, I did take a look and it looks very nice.

Other then that, simple PHP will do the task:


Good 'ol fashioned PHP:

NOTE: I truncate data here, watch out with that and be sure you always pass it through mysql_real_escape_string() to escape it.

$maxlen = 254; // 255 in buffer

if(is_int($_POST['var'])) {
    // it's int.
    $insert = (int) $_POST['var'];
    $sql = "insert into x set x ='".$insert."'";
    $res = mysql_query($sql);

} else {

    // maxlen for collumn
    $var = substr($_POST['var'],0,$maxlen);
    $insert = mysql_real_escape_string($var);
    $sql = "insert into x set x ='".$insert."'";
    $res = mysql_query($sql);
}

Anyway, plenty of ways. though I would stick to mysql_real_escape_string() instead of writing a Regex yourself. A lot of clever people wrote mysql_real_escape_string() and it's considered safe upon strings.



Edited 2 time(s). Last edit at 08/17/2007 10:53AM by Ronald.

Thanks, What you posted is similar to what I have but I was looking for a class which wouldn't mean I'd have to run that script on every variable it do it on the fly.

Do you examples of your custom functions?

I'm building one myself as we speak.

I figured it would be a good idea to build a set of functions or a class to use. Not only for security but I'm also tired -read lazy- to type the same query everytime. So expect a set of functions comming soon.

Yes, sometimes PHP itself can really do the work

To perform numeric input validation (something commonly necessary) for example I usually use ctype_digit($string), which checks if every char from $string is a digit (int)

Ronald, some time ago I was planning to write some validation functions, but decided to wait until I really needed. If you release the class let me know!

Gustavo Cardial



Edited 3 time(s). Last edit at 07/12/2007 12:14AM by gcardial.

Thanks Ronald for the input, yeh if you do finish that mysql class soon please let me know too.

Cheers

There's also the possibilty of cleaning up text variables using str_replace.

After reading a bit about SQL Injections, you can replace all occurrences of UNION or any other SQL statements in your text variables. Something like this:

function CleanText($value)
{
$value = str_replace("UNION","",$value);
$value = str_replace("SELECT","",$value);
$value = str_replace("INSERT","",$value);
$value = str_replace("DELETE","",$value);
return $value;
}

This is an old-style PHP4/MySQL4 prepared statement emulation.
A couple of quick and dirty functions that I put together some years ago.
I kept recycling them for small projects when true prepared statements were not available.
They assume there's just a global "automatic" connection (needed for "real" escaping fitting database charset), but this can be easily adjusted by passing the connection handle around to both or encapsulating them in a class holding the connection as a property.

Typical usage:

mysql_query(
  db_prepare_sql('INSERT INTO mytable (first_name, last_name, phone)
                                values(?, ?, ?)', 
                 $_POST['first_name'], $_POST['last_name'], $_POST['phone']
));

Source code:

function db_prepare_sql($sql, $params) {
  static $replacement = 
    'db_esc($idx < $paramsCount ? $params[$idx++] : null)';

  $numArgs = func_num_args();
  $idx = 0;
  if($numArgs == 1) {
    $paramsCount = 0;
    $idx = 0;
  } else if($numArgs>2 || !is_array($params)) {
    $params = func_get_args();
    $paramsCount = $numArgs;
    ++$idx;
  } else {
    $paramsCount = count($params);
  }
  return preg_replace('/\?/e', $replacement, $sql);
}

function db_esc($unescaped) {
  if(is_array($unescaped)) {
    $res = array();
    foreach($unescaped as $unescapedItem) {
      $res[] = db_esc($unescapedItem);
    }
    return join(',', $res);
  } 
  return ($unescaped === null)
    ? 'NULL'
    : ((is_numeric($unescaped) && !($unescaped{0} === '0' && strlen($unescaped) > 1) )
        ? $unescaped
        : "'" . mysql_real_escape_string($unescaped) ."'")
    ;
}

Cheers

--
*hackademix.net*

There's a browser safer than Firefox... Firefox, with NoScript

Nice ma1, I'm writing something similar and it's almost done. I included formatted table output, and the query and result is wrapped into one function, that maybe to much for some but I thought it would be cool ^^

nvargas Wrote:
-------------------------------------------------------

> function CleanText($value)
> {
> $value = str_replace("UNION","",$value);
> $value = str_replace("SELECT","",$value);
> $value = str_replace("INSERT","",$value);
> $value = str_replace("DELETE","",$value);
> return $value;
> }

note that this function can be circumvented with comments like "UNI/*comment*/ON" in other SQL DBS (not MySQL afaik).

for securing PHP and MySQL I would use the code from php.net:

function check_input($value)
{
// Stripslashes if magic quotes enabled
if (get_magic_quotes_gpc())
{
$value = stripslashes($value);
}
// Quote if not a number
if (!is_numeric($value))
{
$value = "'" . mysql_real_escape_string($value) . "'";
}
return $value;
}

usage:
$cat = check_input($_GET["id"]);
$query = "SELECT id,name FROM users WHERE id = ".$cat."";

bad input wont get rejected, but SQL injection seems to me not possible ;)
length check and checks for special characters can be simply added.

edit:
please let me now if there is any disadvantage of this function, I'm quite new to this topic as well ;)



Edited 1 time(s). Last edit at 07/27/2007 07:59AM by Reiners.

@Reiners

Yes it's almost impossible to RegEx on all SQL injection. I never rely on regexing because you always miss something. Instead encode it properly, escape it and don't try to block it (which could lead to other attacks as well).

So that is good thinking there.

Tried to follow some of the advice here... and got hacked! ;)

I casted all numeric variables like this
$Variable = (int)$_POST['FormVariable'];

Also did string processing with the only string variable like this:

$TextVariable = stripslashes($_POST['FormTextVariable']);
$TextVariable = mysql_real_escape_string($TextVariable);
$TextVariable = substr($TextVariable,0,20);

and still got hacked by a SQL injection. I do want to understand why this didn't work.

@mal:
db_esc() is not correct, you shouldn't use is_numeric for that. Try ".1" for example.

@Reiners:
check_input() is not correct, it assumes the input comes from one of the gpc arrays and it also uses is_numeric incorrectly.

@nvargas:
Move the substr() line above the mysql... line, that's your bug.
Of course there can be a ton more, untill you show us code noone can say ;)

Also, doing stripslashes like that is not correct (although I can't think of any security implications).

Mordred,

Ronald pointed that I should also cast and escape all query outputs. I'm doing that, checking if the script is still vulnerable.

Did a few trial runs injecting SQL and what I got is this particular case was the injection string saved into the database, but the injected query string didn't work.

Most of my code, variables and comments are in spanish, that's why I didn't submit it, plus the script only takes 4 variables, 3 of them are numerical and one is string. All numerical variables were casted, so my best guess is that the string variable is the culprit.

I'm not using cookies or $_SERVER variables, so I don't think that is an issue.

I mailed you.

This is very important, because in my example I first check the size and truncate it before I pass it into mysql_real_esacpe_string(). Security is a process, step by step.

So watch out with that.

Mordred Wrote:
-------------------------------------------------------
> @Reiners:
> check_input() is not correct, it assumes the input
> comes from one of the gpc arrays and it also uses
> is_numeric incorrectly.

Hi Mordred!
thanks for your input. Unfortunetly I dont see your points :(
can you please be more specific about what is wrong with "is_numeric" and why I use the function wrong with $_GET[] ?

I just wanted to share a scenario I thought of when I read nvargas code and Mordred suggestion:

//code

$name = stripslashes($_GET['name']); 
$name = mysql_real_escape_string($name); 
$name = substr($name,0,20);

$id = $_GET['id']; 

$query = "SELECT name,data FROM users WHERE name = '".$name."' AND id = '".$id."'";

//attack
site.com/sql.php?name=1234567890123456789'&id=%20OR%201=1/*
mysql_real_escape_string --> 1234567890123456789\'
substr(20) --> 1234567890123456789\

//query in the end
SELECT name,data FROM users WHERE name = '1234567890123456789\' AND id = ' OR 1=1/*'

(note that the second single quote is escaped and the name ends at the third single quote)

I dont want to bore the one or the other ;) I played with the code before reading closely that nvargas casts numeric variables, but since its quite different to normal injections I decided to post anyways ;)

Well, is_numeric() is a tricky function.
I rather cast, it's faster and safer.

Rainers, yep, you got the problem with substr right :)

As for that check_input:

"check_input() is not correct, it assumes the input comes from one of the gpc arrays and it also uses is_numeric incorrectly."

Here's the rub: if you have magic quotes on, and you do check_input($_GET['blah']), it is okay, as the stripslashes will undo the magic quotes effect.
If you do it on a values that comes from somewhere else though:
$sVal = "Here's a slash and a quote: \\'";
check_input($sVal) will incorrectly strip the slash and return just the quote.

is_numeric() has several pitfals, see the manual for details, but in short:
is_numeric('0.172') == true
is_numeric('0xBADC0DE') == true
etc..

thanks Mordred, now I see your points.

Well for the check_input() I will remember to use it only for user input (Get/Post), but thats what it was designed for anyways I guess. Maybe I'll rename it to check_gpc_input() ;). First, I thought of checking if the $value is in one of the gpc arrays, but it can always be added there easily, so that makes no sense.

for the is_numeric():
I can't think of a way to exploit this (even with different sql queries). Floating point numbers passes but I can't do anything interesting with them, can I ? The same for hexnumbers. I dont see an attack vector with 6 letters (A-F) and still would have to get rid of the "0x" at the beginning. Or am I missing something?
Anyways its better to cast numbers I guess.



Edited 1 time(s). Last edit at 08/18/2007 07:19PM by Reiners.

I followed your advice and still got injected. This time he managed to inject a javascript string like this: <script> document.write('xyz was here'); </script>

I thought that truncating, casting and escaping would do the job, but it seems not. I really want to understand why it gets hacked.

I sanitized my variables like this:

$Accion = (int)$_POST['accion'];
$Encuesta = (int)$_POST['Encuesta'];
$Opcion = (int)$_POST['Opciones'];
$TextoOtro = substr($_POST['txtOtro'],0,50);
$TextoOtro = mysql_real_escape_string($TextoOtro);
$TextoOtro = strip_tags($TextoOtro);

Since the string is escaped, I guess there are not html tags embedded, but I didn't check for url encoded strings. Those are the ONLY variables I read from any external source. Following Ronald's advice, I also escape and cast database fetches.

The url to the site is [www.canal2tv.net]

I tried to exploit myself, to no avail, I only managed to get my exploit string saved into the database as a regular escaped string. Obviously nvargas.exploiting_skills = NULL

Any advice or reading is very welcome.

Thanx in advance

Does print_poll.php strip slashes?, please post the full code for review the above code works just fine and isn't vulnerable. It might be an output problem.

Always output: htmlspecialchars($stuff,ENT_QUOTES,'UTF-8') or htmlentities() and on't stripslashes cause it removes the escaping you put there.

"Sanitizing" input is a subtly wrong concept, there is no general way to "make things safe". You, for example, try to prepare data for database and html usage at once. This is wrong, if you're going to put that in the database, prepare it as such. Later, when you want to display it, prepare it for displaying. Never do it at once.

mysql_real_escape_string() should be the LAST thing run on the string before it is passed to mysql_query(). The previous time with substr(), it was fatal; here it looks like it isn't, but it could have been ;)

So the snippet you posted looks okay, but the vulnerability (or not) of the script would depend on the other pieces of code - the one actually putting this in the database and the one that displays it.

Ok, here's the whole enchilada. Since I'm coding in spanish, hence the comments

$dbConexion = mysql_pconnect($Servidor,$Usuario,$Contrasena) or die('No se pudo conectar al servidor');
$dbBaseDatos = mysql_selectdb($BaseDatos) or die('No se pudo abrir base de datos');
$Accion = (int)$_POST['accion'];
$Encuesta = (int)$_POST['Encuesta'];
$Opcion = (int)$_POST['Opciones'];
$TextoOtro = substr($_POST['txtOtro'],0,50);
$TextoOtro = mysql_real_escape_string($TextoOtro);
$TextoOtro = strip_tags($TextoOtro);

/* Si ya votó, que pase a sumar y mostrar los resultados */

if($Accion == 1){

//Vemos cuantos votos tiene la opcion actualmente y la incrementamos en uno

$stSQL = "SELECT intVotos,bytOtro FROM tblVotos WHERE idEncuesta = ".$Encuesta. " AND intOpcion = ".$Opcion. ";";
$Resultado = mysql_query($stSQL) or die('Error de acceso');
if($Resultado) {
$DatosResultado = mysql_fetch_assoc($Resultado);
$Votos = (int)$DatosResultado['intVotos'] + 1;
$TipoOtro = (int)$DatosResultado['bytOtro'];
}
else
die('Error en la encuesta');
mysql_free_result($Resultado);

//Actualizar la base de datos de votos

$stSQL = "UPDATE tblVotos SET intVotos = ".$Votos." WHERE idEncuesta = ".$Encuesta." AND intOpcion = ".$Opcion.";";
mysql_query($stSQL) or die('Error de Actualizacion');

//Si el voto era OTRO, actualizar el dato

if($TipoOtro == 1 && $TextoOtro!=""){
//Verificar que no exista
$stSQL = "SELECT intVotos FROM tblOtros WHERE idEncuesta = ".$Encuesta." AND intOpcion=".$Opcion." AND txtTexto ='".$TextoOtro."';";
$Resultado = mysql_query($stSQL) or die('Error de lectura');
//Si no existe
if(mysql_numrows($Resultado)==0){
$stSQL = "INSERT INTO tblOtros (idEncuesta, intOpcion,txtTexto,intVotos) VALUES (".$Encuesta.",".$Opcion.",'".$TextoOtro."',1);";
mysql_query($stSQL) or die('Error de Insercion');
}
else {
$DatosResultado = mysql_fetch_assoc($Resultado);
$Votos = (int)$DatosResultado['intVotos'] + 1;
$stSQL = "UPDATE tblOtros SET intVotos = ".$Votos." WHERE idEncuesta = ".$Encuesta." AND intOpcion = ".$Opcion." AND txtTexto = '".$TextoOtro."';";
mysql_query($stSQL) or die('Error de actualizacion');
};
mysql_free_result($Resultado);
}

// Mostrar los resultados

$stSQL = "SELECT tblEncuestas.txtTitulo,sum(tblVotos.intVotos) AS TotalVotos FROM tblEncuestas INNER JOIN tblVotos ON tblEncuestas.idEncuesta = tblVotos.idEncuesta WHERE tblEncuestas.idEncuesta = ".$Encuesta." GROUP BY tblEncuestas.idEncuesta;";
$Resultado = mysql_query($stSQL) or die('Error de lectura');
if(mysql_numrows($Resultado)>0){
$DatosResultado = mysql_fetch_assoc($Resultado);
$Total = (int)$DatosResultado['TotalVotos'];
$Titulo = mysql_real_escape_string($DatosResultado['txtTitulo']);
}
else {
$Total = 0;
$Titulo = "Encuesta";
};
mysql_free_result($Resultado);
$stSQL = "SELECT txtTitulo,intVotos FROM tblVotos WHERE idEncuesta = ".$Encuesta.";";
$Resultado = mysql_query($stSQL) or die('Error de lectura');
if($Resultado){
echo "<table width=\"346\" height=\"148\" border=\"0\" align=\"center\" cellpadding=\"0\" cellspacing=\"0\" bgcolor=\"#FFFFFF\">\n";
echo "<tr><td height=\"135\" valign=\"bottom\" bgcolor=\"#FFFFFF\" class=\"Caja_titulo\"><strong class=\"Encuesta_titl\">".$Titulo."<br>\n";
echo "<table border=\"0\" border=\"0\" cellpadding=\"0\" cellspacing=\"0\" width=\"346\">";
while($DatosResultado = mysql_fetch_assoc($Resultado)){
echo "<tr class=\"Encuesta_items\"><td width=\"100\">".mysql_real_escape_string($DatosResultado['txtTitulo'])."</td><td width=\"100\"><img src=\"http://www.canal2tv.com/encuestas/rojo.gif\" width=\"".round(((int)$DatosResultado['intVotos'])*100/$Total)."\" height=\"10\"></td><td>(".(int)$DatosResultado['intVotos']." - ".number_format(((int)$DatosResultado['intVotos'])*100/$Total,2)."%)</td></tr>\n";
}
echo "</table>\n";
echo "</td></tr></table>\n";
echo "<p class=\"Encuesta_items\">TOTAL DE VOTOS: ".$Total."</p>";
}
mysql_free_result($Resultado);
}
else {
echo "<form id=\"frmEncuesta\" name=\"frmEncuesta\" method=\"post\" action=\"print_poll.php\">\n";
echo "<table width=\"346\" border=\"0\" align=\"center\" cellpadding=\"0\" cellspacing=\"0\" bgcolor=\"#FFFFFF\">\n";
$Fecha = getdate();
$CadenaFecha = $Fecha[year]."-".$Fecha[mon]."-".$Fecha[mday];
$stSQL = "SELECT txtTitulo,idEncuesta FROM tblEncuestas WHERE datFechaInicio <= '".$CadenaFecha."' and datFechaFin >= '".$CadenaFecha."' ORDER BY idEncuesta;";
//$stSQL = "SELECT txtTitulo,idEncuesta FROM tblEncuestas WHERE datFechaInicio <= '2007-08-15' and datFechaFin >= '2007-08-15' ORDER BY idEncuesta;";
$Resultado = mysql_query($stSQL) or die('Error de lectura');
if($Resultado) {
$DatosResultado = mysql_fetch_assoc($Resultado);
if(mysql_numrows($Resultado) > 0) {
$Encuesta = (int)$DatosResultado['idEncuesta'];
$Titulo = mysql_real_escape_string($DatosResultado['txtTitulo']);
echo "<tr><td height=\"135\" valign=\"bottom\" bgcolor=\"#FFFFFF\" class=\"Caja_titulo\"><strong class=\"Encuesta_titl\">".$Titulo."<br>\n";
mysql_free_result($Resultado);
$stSQL = "SELECT intOpcion,txtTitulo,bytOtro FROM tblVotos WHERE idEncuesta = ".$Encuesta." ORDER BY intOpcion;";
$Resultado = mysql_query($stSQL) or die('Error de lectura');
while($DatosResultado=mysql_fetch_assoc($Resultado)){
echo "<label class=\"Encuesta_items\"><input name=\"Opciones\" type=\"radio\" value=\"".(int)$DatosResultado['intOpcion']."\" >".mysql_real_escape_string($DatosResultado['txtTitulo'])."</label>";
// Si la opcion es de tipo OTRO imprimir esto
if((int)$DatosResultado['bytOtro']==1){
echo " <input name=\"txtOtro\" type=\"text\" id=\"txtOtro\" size=\"20\" maxlength=\"50\">\n";
};
echo "<br>\n";
};
}
else die('No hay encuesta activa');
mysql_free_result($Resultado);
echo "<input name=\"accion\" type=\"hidden\" value=1>";
echo "<input name=\"Encuesta\" type=\"hidden\" value=".$Encuesta.">";
echo "<input name=\"cmdVotar\" type=\"submit\" id=\"cmdVotar\" value=\"Votar!\">";
echo "</tr></table>\n";
echo "</form>\n";
};
};
mysql_close($dbConexion);

It seems that TextoOtro is handled safely here, is this where the hack you report originated? Which of the text fields you pull from the database could be controlled by an attacker? You don't have XSS protection here, so if any of the other strings may come from the attacker, you're toast.

Read my previous post again and answer to yourself (write here if you're not sure of the answer) what does this (and the other such lines) do: $Titulo = mysql_real_escape_string($DatosResultado['txtTitulo']);

Don't do this:

echo "<label class=\"Encuesta_items\"><input name=\"Opciones\" type=\"radio\" value=\"".(int)$DatosResultado['intOpcion']."\" >
".mysql_real_escape_string($DatosResultado['txtTitulo'])."</label>";

That is bad coding and is useless because it already was escaped when you inserted data into the db. Depending on magic_quotes and this stuff can put \\\\\\\\' before a single quote, and escaping the slashes also. Just use:

htmlspecialchars($DatosResultado['txtTitulo'],ENT_QUOTES);

And you should be fine.

After doing a bit of reading, I'm thinking it was a bit of both: XSS and SQL Injection.

The guy left a <script>document.write('XYZ was here');</script> on my database, on the Poll title, that's why I suppose it was a SQL Injection, since the poll title is on a different table to the one I use to register votes.

Now that Mordred pointed out, there's no XSS protection at all, so I'm doing a bit of research on my own about it.

Any reading URLs you can provide will be greatly appreciated.

Like I said:

htmlspecialchars($DatosResultado['txtTitulo'],ENT_QUOTES);

There is nothing magical about it.