[Phpwm] site critique please

[Ian Munday]

On 16 Jan 2007, at 19:14, Phil Beynon wrote:

>>> Hi David,
>>>
>>> How?
>>> All that's making it do is throw a MySQL error and immediately exit;
>>>
>>> $result1 = mysql_query("SELECT * FROM page_content WHERE id =
>> '$ident';");
>>> if(!$result1){echo("<p>Error performing query: " .
>> mysql_error() . "</p>");
>>> exit();}
>>>
>>> There's a couple of variables that come from the siteconfig,
>> but these would
>>> overwrite anything injected due to when they are read in.
>>
>> Hi,
>>
>> Right, I'm no elite security expert, however as far as I  
>> understand it
>> is a big no no no never ever ever include variables from any user
>> supplied input directly into an SQL statement.
>>
>> For instance (and not being able to read your code, I may be  
>> wrong) you
>> could be something like :
>>
>>
>> $ident = $_GET['ident'];
>> $result1 = mysql_query("SELECT * FROM page_content WHERE id =  
>> '$ident';");
>> // etc.
>>
>> In this case, anything I put into ident= on the URL will be passed to
>> MySQL. I could therefore do something like :
>>
>> http://foo.bar/whatever.php?ident=12';DROP DATABASE;'
>
> Which shouldn't actually work and should be rejected by MySQL as the
> database login doesnt have that as a priviledge.
>
>> That would then get executed as 'DROP DATABASE' within MySQL... hence
>> it's a problem.
>>
>> You MUST either :
>>
>> 1) Use prepared statements (either via PEAR::DB, PEAR::MDB2 or  
>> mysqli)
>> or
>> 2) Run mysql_escape_string() ON ALL user supplied input.
>> or
>> 3) Use a higher level framework (e.g. Propel) that does
>> escaping/sanitisation for you.
>
> I shall indeed harden up the permissible inputs with some regex! :-)
>
> I'll post back what goes in there for critique and assistance to other
> users!
> In the case of this one stripping out anything non numeric should  
> suffice.
>
>> There are far more (and better) Examples of SQL injection; as a start
>> try reading this article from LWN :
>>
>> http://lwn.net/Articles/177037/
>>
>
> I shall read it! Anything that hardens sites up is good in my book!

I used a 1.x version of SafeSQL  -  http://www.phpinsider.com/php/ 
code/SafeSQL/  -  which I later adapted slightly.  I've moved on to  
using prepared statements via PEAR::MDB2 now for various reasons, but  
not because the SafeSQL method wasn't satisfactory.

SafeSQL always handled both simple and complex requirements pretty  
elegantly.

The synopsis gives a simple example:

     require 'SafeSQL.class.php';

     // dummy up a variable with a single quote in it
     $section_name = "fred's place";

     // run the query through SafeSQL
     $safesql =& new SafeSQL_MySQL;
     $query_string = $safesql->query("select * from sections where  
Section_Name = '%s'", array($section_name));

     echo $query_string;

     OUTPUT:
     select * from sections where Section_Name = 'fred\'s place'

     // $query_string is now safe to pass to your SQL library


Ian


----
Ian Munday
Illumen Ltd.
www.illumen.co.uk

Tel:  0121 703 3111  /  0845 00 999 00
Email:  ian.munday at illumen.co.uk
Skype:  ian.munday