LebGeeks

proners

Member

creation of argument safe database_query without preg callback | PHP

So i have my own database_query function that automatically sanitizes arguments, it is very helpful(less typing in future code ) but its only downside that it relies on preg_replace_callback to sanitize the arguments, preg function are proven to be slow especially when having 200+ queries per page.

So i have started brainstorming to create a new one

function database_query($query) {
  $args = func_get_args();
  $original_query = array_shift($args); // remove first array entry
  $keys = $original_query;
  $keys = explode('%', $keys);
  array_shift($keys);
  foreach($keys as &$el){$el = substr((string)$el,0,1);}
  if (isset($args[0]) and is_array($args[0])) { 
  	// Support for 'All arguments in one array' syntax
    $args = $args[0];
  }
  print_r($keys);
  print_r($args);
  
  $new_query = "\"".$original_query."\", ";
    
  if(isset($args) && count($args)!=0){
  	$args = array_combine($keys, $args);
  	
  	foreach($args as $key => &$value){
      switch ($key) {
        case 'd': 
          $new_query .= $value.", "; break; 
        case 's':
          // WARNING: i am only using urlencode for testing without database interaction
          // for testing with a database, replace urlencode with mysql_real_escape_string for example
          $new_query .= "\"".urlencode($value)."\", "; break;
        case 'n':
          // Numeric values have arbitrary precision, so can't be treated as float.    
          $new_query .= is_numeric($value) && !preg_match('/x/i', $value) ? $value : '0';
          //well is_numeric() allows hex values (0xFF) but not valid.
          $new_query .= ", ";
          break;
        case 'f':
          $new_query .= (float)$value.", "; break;
        case 'b': // binary data
          $new_query .= "'".urlencode($value)."', "; break;
      }
    }
    $query = substr($new_query, 0, -2); //remove ", " from end!!!
    echo $query // for testing
    
    // NOW HERE I AM GETTING AN ERROR for sprintf, too few arguments
    $query= printf($query);
    //mysql_query($query) if you are testing with database
   	
  	
  }
  else{
  	$query = $query; // if no arguments   
  	echo $query;
  	//mysql_query($query) if you are testing with database
  }
}

//As a test
$uid = 5;
$name = "<prone";
database_query("SELCT FROM node WHERE uid=%d AND name=%s", $uid, $name);

so feel free to improve this code or propose a new approach, until i see this through, i'll stick database_query with preg_replace_callback, maybe the final code will be bloated  to be slower than the first one, but oh well it's nice to have choices

rolf

Member

Re: creation of argument safe database_query without preg callback | PHP

database_query("SELECT FROM node WHERE uid=%d AND name=%s", $uid, $name);

Thats pretty cool...

Im more of a

$uid = check_digits($uid);
$name = check_string($name);
mysql_query("SELECT FROM node WHERE uid=$uid AND name=$name");

guy

:)

Padre

Member

Re: creation of argument safe database_query without preg callback | PHP

Last time I had to write php and use SQL I just used str_replace($not_valid,"",$my_qry)
Or smth similar, not very sure about the syntax

samer

Admin

Re: creation of argument safe database_query without preg callback | PHP

I'm used to using preg replace as well, which as you pointed out is not the most efficient solution :)

rolf

Member

Re: creation of argument safe database_query without preg callback | PHP

I think the best would be to do data check on the client side using javascript, then just use mysql_escape for code injection security.

nuclearcat

Member

Re: creation of argument safe database_query without preg callback | PHP

Want fast?
Check uid by is_numeric()
name by ctype_alpha()

samer

Admin

Re: creation of argument safe database_query without preg callback | PHP

I think the best would be to do data check on the client side using javascript

That would be useless from a security point of view.

proners

Member

Re: creation of argument safe database_query without preg callback | PHP

@nuc, what i am re-writing is a function that prevents sql injection
for example
database_query("SELECT (stuff ...) FROM TABLE WHERE stuff1='%s' stuff2 ='%s' stuff3=%d ...", $arg1, $arg2, $arg3 ...)
so the arguments are passed and safely checked(for numeric values) and escaped before passed directly to the query

what i am using now is preg_replace_callback inside the database_query function that makes a call(surprise! /P) for another function that replaces safely the arguments one by one before database_query can call mysql_query

so what you're saying is actually not the fast way, it's the long way around, because you'd have to do these checks for every query but when you have a function that does that for you, you don't bother anymore and just get thinking about more important stuff...

@rolf, I'd agree with samer, javascript is useless from security point of view,
it's used to relieve pressure/load from the server so it won't have to do the checks multiple times before the user get it RIGHT  but only once supposing that he's not doing this intentionnaly to find a weak point...

Last edited by proners (May 10 2009)

nuclearcat

Member

Re: creation of argument safe database_query without preg callback | PHP

Sure u can make a function and parse array where is defined what type each attribute should be.

Most important IMHO just to drop escape symbols (mysql_real_escape_string and similar, as mentioned before) and provide correct query that cannot be mangled in wrong way(e.g. ...' password LIKE "'.$data.'" '... is insecure, because % can be used).

rolf

Member

Re: creation of argument safe database_query without preg callback | PHP

proners and samer, I dont see what is unsafe. I said use mysql_escape on the server, it will stop all code injection attempts because it will escape any non-mysql characters...
i dont see what other security problems can arise. If your db access is properly coded, nothing should be a problem.
If the client bypasses javascript and sends inconsistent, data, then all what should happen is the database saving garbage.

mysql> create table blekh (dob INT);
Query OK, 0 rows affected (0.02 sec)

mysql> select * from blekh;
Empty set (0.00 sec)

mysql> insert into blekh (dob) values ('astring');
Query OK, 1 row affected, 1 warning (0.00 sec)

mysql> select * from blekh;
+------+
| dob  |
+------+
|    0 |
+------+
1 row in set (0.00 sec)

See, in the example above, a garbled data was sent and all what happened is the field dob (for date of birth) was set to 0.

It is a not a security problem... Its just garbage in, garbage out... and it normally should never happen with javascript checks.

Last edited by rolf (May 10 2009)

proners

Member

Re: creation of argument safe database_query without preg callback | PHP

@nuc, yeah you're right, and that's what i am trying to achieve in most cases
But another layer of security is needed when using "LIKE"(grant or revoke for that matter) in the query, i usually use addcslashes(escaping every char :O ) for them or preg_replace the %_ , but i rarely use them so i pay extra attention when i do. but in the usual queries(at least usual for me), a function that checks types and parses and replaces works well...

@rolf, well basically what you're using prevents injections, but another layer of validation on the server is needed IMHO, garbage is stinky, so why put it there in the first place

proners

Member

Re: creation of argument safe database_query without preg callback | PHP

well anyway injections are the least of the problems a web developper thinks about, filtering output to prevent xss is much more complex because of the quirks of IE(especially 6)

rolf

Member

Re: creation of argument safe database_query without preg callback | PHP

well anyway injections are the least of the problems a web developper thinks about, filtering output to prevent xss is much more complex because of the quirks of IE(especially 6)

htmlentities()

I dont know why youre not using embedded php functions to escape sql and html. As for type checking (date, numbers...) i think they should be replaced with javascript on the client side.

The thing I like about your approach, is the syntax.

Why not replace %d, %s, etc... with %text and %html for example

proners

Member

Re: creation of argument safe database_query without preg callback | PHP

I dont know why youre not using embedded php functions to escape sql and html. As for type checking (date, numbers...) i think they should be replaced with javascript on the client side.

The thing I like about your approach, is the syntax.

Why not replace %d, %s, etc... with %text and %html for example

Well we do agree that

database_query("INSERT INTO TABLE (s1, s2, s3, s4) VALUES(%d, '%s', '%s', %b)", $arg1, $arg2, $arg3, $arg4);

is better than

if(!is_numeric($arg1)){$arg1 = 0;} //well just for the sake of fastness, but i usually i do have a check that returns the form to the user so he would correct the input
$arg2= mysql_real_escape_string($arg2);
$arg3= mysql_real_escape_string($arg3);
$arg4= "'".mysql_real_escape_string($arg4)"'";
mysql_query(sprintf("INSERT INTO TABLE (s1, s2, s3, s4) VALUES(%d, '%s', '%s', %b)", $arg1, $arg2, $arg3, $arg4), $connection);

;) cheezy syntax

now about your question i use the functions provided by mysql(mysql_real_escape_string) because imo it's the most optimized for this, most importantly when i use it to insert values, i can SELECT those values back and mysql sends them to me in their UNESCAPED form ;)

anyway here's a warning for anyone reading this forum maybe as a reference
when using mysql_real_escape string do not write code like this

mysql_query(sprintf("INSERT INTO TABLE (s1, s2, s3, s4) VALUES(%d, `%s`, `%s`, %b)", $arg1, $arg2, $arg3, $arg4), $connection);

but

mysql_query(sprintf("INSERT INTO TABLE (s1, s2, s3, s4) VALUES(%d, '%s', '%s', %b)", $arg1, $arg2, $arg3, $arg4), $connection);

because mysql_real_escape_string  doesn't escape the char(`), but it does escape the single quote

Last edited by proners (May 10 2009)

rolf

Member

Re: creation of argument safe database_query without preg callback | PHP

I have rewritten the function

<?php
function db_query() {
    $sep = "%";
    $args = func_get_args();
    $q = array_shift($args);
    $nq = '';

    foreach (explode($sep, $q) as $i => $tok) {
        // switch the token only if there are remaining arguments, otherwise switch an empty string which will go to default.
        // Please do not define an empty string ('') case...!
        switch ( ($rem_args = count($args)) > 0 ? $tok : '' ) {
           case 'text':
               $nq .= htmlentities(array_shift($args));
           break;
           case 'html':
               $nq .= htmlentities(array_shift($args));
           break;
           case '':
           default:
               // if this is not the 1st token, and if previous token was not replaced, then keep the separator
               // its ugly but necessary ... 
               ($i > 0) and ($rem_args_old == $rem_args) and $nq .= $sep;
               $nq .= $tok;
               $rem_args_old = $rem_args;
           break;
        }
    }
    return $nq;
    // return mysql_query($nq);
}

// testing (i know, invalid SQL)
$name = 'meh>meh';
echo db_query('%SELECT * FROM table WHERE %blabla test %heh% name="%text%" AND name2=%html% AND name3=%text%',$name, $name);

?>

The function will replace %text%, %html% and any other thingie you define. If it doesnt recognize the pattern (example %text , %undefinedkeyword%, or single % somewhere) or if arguments are exhausted, it will not do any replacement.

Last edited by rolf (May 11 2009)

proners

Member

Re: creation of argument safe database_query without preg callback | PHP

i've tried adding

           case 'number':
             $n = array_shift($args);
             $nq .= is_numeric($n)? $n : "0";

it didn't work well ...

proners

Member

Re: creation of argument safe database_query without preg callback | PHP

okay since you were generous enough to invest some of your time and take a stab at it, here's my newest version of the function

function database_query() {
  $args = func_get_args();
  $original_query = array_shift($args); // remove first array entry
  if (isset($args[0]) and is_array($args[0])) { 
      // Support for 'All arguments in one array' syntax
    $args = $args[0];
  }
  $keys = $original_query;
  $keys = explode('%', $keys);
  array_shift($keys);
  $i = 0; 
  foreach($keys as &$el){
  	$i++; 
    $el = substr((string)$el,0,1)."$i"; // concatenated $i to make unique keys 
  }
  //print_r($keys);

  
  $new_args = array();
    
  if(isset($args) && count($args)!=0){
      $args = array_combine($keys, $args);
      print_r($args);
      
      foreach($args as $key => $value){
 	  $key = substr($key, 0, 1);
      switch ($key) {
        case 'd': 
          $new_args[] = $value; break; 
        case 's':
          // WARNING: i am only using urlencode for testing without database interaction
          // for testing with a database, replace urlencode with mysql_real_escape_string for example
          $new_args[] = urlencode($value); break;
        case 'n':
          // Numeric values have arbitrary precision, so can't be treated as float.    
          $new_args[] = is_numeric($value) && !preg_match('/x/i', $value) ? $value : '0';
          //well is_numeric() allows hex values (0xFF) but not valid.
          break;
        case 'f':
          $new_args[] = (float)$value; break;
        case 'b': // binary data
          $new_args[] = "'".urlencode($value)."'"; break;
      }
    }
    
    $query = vprintf($original_query, $new_args);
    //mysql_query($query) if you are testing with database
       
      
  }
  else{
      $query = $query; // if no arguments   
      echo $query;
      //mysql_query($query) if you are testing with database
  }
}

//As a test
$uid = 5;
$name = "name";
database_query("SELECT FROM node WHERE text='%s' AND uid=%d AND name='%s'", $name, $uid, $name);

well i dropped printf since there was no way it was going to work with it and used vprintf instead  and i fixed the issue of array_combine overlapping values because keys needs to be unique so i made them so
now it's working correctly..

Last edited by proners (May 11 2009)

rolf

Member

Re: creation of argument safe database_query without preg callback | PHP

It is better, but...
- Keep it simple, I would suggest u use my code.
- I am against type checking for integers and such, it must be done at the javascript level. And mysql does the type casting anyway.
- There is no default case in your switch, if the % is part of the sql syntax or if the letter after it is unexpected, then the behavior of your function is undefined.

i've tried adding

           case 'number':
             $n = array_shift($args);
             $nq .= is_numeric($n)? $n : "0";

it didn't work well ...

Did you write a break to close the case?

:rolleyes:

Last edited by rolf (May 11 2009)

proners

Member

Re: creation of argument safe database_query without preg callback | PHP

It is better, but...
- Keep it simple, I would suggest u use my code.

Well for the time being i am not using the one i created nor yours, i have another one that is working perfectly so until i do extended testing on both, i won't risk to use them. but thanks :)

- I am against type checking for integers and such, it must be done at the javascript level

i think you ought to check types at both levels, html and javascript can be manipulated(doh) so one can send you text in a input that you've expected to be a number therefore an injection is likely.

- There is no default case in your switch, if the % is part of the sql syntax or if the letter after it is unexpected, then the behavior of your function is undefined

yes you are 100 per cent right, i can't use YET % as a part of mysql syntax in the current function, but i am going to do something about it ;)
and imo you shouldn't allow the input of more or less arguments(parameters) than required because you'll have an invalid query or you might mask some nasty bugs this way in other parts of your code that you won't be able to pinpoint fastly...

edit: with break; it works ... silly

Last edited by proners (May 11 2009)

rolf

Member

Re: creation of argument safe database_query without preg callback | PHP

i think you ought to check types at both levels, html and javascript can be manipulated(doh) so one can send you text in a input that you've expected to be a number therefore an injection is likely.

mysql_real_escape = no injection

and imo you shouldn't allow the input of more or less arguments(parameters) than required because you'll have an invalid query or you might mask some nasty bugs this way in other parts of your code that you won't be able to pinpoint fastly...

yeah, i think youre right. no biggie though, 2 extra lines of codes should do it.

Well for the time being i am not using the one i created nor yours, i have another one that is working perfectly so until i do extended testing on both, i won't risk to use them. but thanks :)

less code = less bugs

Last edited by rolf (May 11 2009)

proners

Member

Re: creation of argument safe database_query without preg callback | PHP

well, i made a quick fix for using % in a query, i had to use preg_replace :| meh

function database_query() {
  $args = func_get_args();
  $q = array_shift($args); // remove first array entry
  if (isset($args[0]) and is_array($args[0])) { 
      // Support for 'All arguments in one array' syntax
    $args = $args[0];
  }
  $keys = explode('%', $q);
  array_shift($keys);
  $i = 0; 
  foreach($keys as &$el){
    $i++; 
    $el = substr((string)$el,0,1)."$i"; // concatenated $i to make unique keys 
  }
    
  if(isset($args) && count($args)!=0){
      $args = array_combine($keys, $args);
      
      foreach($args as $key => &$value){
       $key = substr($key, 0, 1);
      switch ($key) {
        case 'd': 
          $value = $value; break; 
        case 's':
          // WARNING: i am only using urlencode for testing without database interaction
          // for testing with a database, replace urlencode with mysql_real_escape_string for example
          $value = urlencode($value); break;
        case 'n':
          // Numeric values have arbitrary precision, so can't be treated as float.    
          $value = is_numeric($value) && !preg_match('/x/i', $value) ? $value : '0';
          //well is_numeric() allows hex values (0xFF) but not valid.
          break;
        case 'f':
          $value = (float)$value; break;
        case 'b': // binary data
          $value = "'".urlencode($value)."'"; break;
      }
    }
    $q = preg_replace('/\(pc\)/', '%%', $q);
    $q = vsprintf($q, $args);
    
    echo $q;

       
      
  }
  else{
      echo $q;
      //mysql_query($q) if you are testing with database
  }
}

//As a test
$uid = 5;
$name = "name";
$name1 = 'dido<d>';
database_query("SELECT FROM node (pc)WHERE text='%s' AND uid=%d AND name='%s'", $name1, $uid, $name); // (pc) is replaced by %.. dammit i had to use preg_replace after all :/

less code = less bugs

That's not necessarily true...

mysql_real_escape = no injection

okay so you're escaping numbers, hmmm... well you're happy with it

also a follow up for a previous question, you asked why i use mysql_real_escape_string instead of htmlentities for escaping html, well when you use htmlentities you do escape html but you won't be able to make other operations on it like strip_tags and you'll have those ugly tags showing in the comments on your website for example, or you'd have to decode, do operations, then encode which is quite unnecessary... i treat html as any normal text when entering to the database, the philosophy that i believe in is to store in as is(if type is correct) and filter out nasty stuff(e.g. xss attacks) on output

well i'll do some extended testing on these two functions, and benchmark them(when i have free time) and the current one i am using... w it's the only objective way to make a proper judgement, well anyway thanks again :) it's cool to have multiple options  this thread had been/is a great fun(will continue to be )... i am going to open another thread for discussing how to prevent xss attacks http://www.lebgeeks.com/forums/viewtopi … 744#p22744, great to see others experiences in this subject

Last edited by proners (May 11 2009)

rolf

Member

Re: creation of argument safe database_query without preg callback | PHP

okay so you're escaping numbers, hmmm... well you're happy with it

$n = 12 ;
mysql_query( 'SELECT * FROM users WHERE user="'.mysql_real_escape($n).'"' );

whats wrong with that?

Last edited by rolf (May 12 2009)

proners

Member

Re: creation of argument safe database_query without preg callback | PHP

whats wrong with that?

nothing actually, it's just a different philosophy/way of work