4

I am using ADOdb for connecting to my MSSQL database. I am wondering is this good enough to prevent SQL injection?

The prepared query that I am using is:

       $db = ADONewConnection('odbc_mssql');
       $dsn = "Driver={SQL Server};Server=SERVNAME;Database=DBNAME;";
       $ADODB_COUNTRECS = false;

       $db->Connect($dsn,'LOGIN','PASS');

       $sql = 'SELECT login, etc FROM users WHERE login ='.$db->Param('0').' AND pass ='.$db->Param('1').'';
       $stmt = $db->Prepare($sql);  
       $stmt = $db->Execute($stmt,array("$user_id","$psw"));

OR would you perhaps recommend switching to PDO?

GEMI
  • 2,239
  • 3
  • 20
  • 28

1 Answers1

7

I do not understand why nobody tried to answer this, or even comment in any way. SO works in mysterious ways I guess.. ;) Anyhow, I have researched enough (I think so) to be able to answer this myself, with a certain degree of confidence.

So in essence prepared queries are much safer than just input escaping such as mysql_real_escape_string(); , because as some wise people have said:

Yes, mysql_real_escape_string is effectively just a string escaping function. It is not a magic bullet. All it will do is escape dangerous characters in order that they can be safe to use in a single query string. However, if you do not sanitize your inputs beforehand, then you will be vulnerable to certain attack vectors.

Link to full answer: mysql_real_escape_string

So what I have done to test my prepared statement vs escaping - I have made a simple submit form and tried to sanitize inputs with mysql_real_escape_string(); and surely it failed with examples like "1 OR 1=1", some people suggested adding the escaped values inside single quotes like that:

$query = "SELECT * FROM mydb WHERE ID = ' ".$escapedID." ' ";

It helps to prevent the "1 OR 1=1" example, but surely this is not the best practice. The problem with escaping is that it does not protect from people altering your query logic in malicious ways.

So from now on I will stick to ADODB prepared statements. Like the one in my original question above, or a shorter version from bobby-tables website:

   $dbConnection = NewADOConnection($connectionString); 
   $sqlResult =  $dbConnection->Execute( 
   'SELECT user_id,first_name,last_name FROM users WHERE username=? AND password=?'
   , array($_POST['username'], sha1($_POST['password']) );

Other very helpful questions regarding SQL Injections:

Also watch this video, it makes SQL injection a lot easier to grasp:

Community
  • 1
  • 1
GEMI
  • 2,239
  • 3
  • 20
  • 28
  • From your reasoning one can't say that you understood the matter properly. 1. single quotes is not to protect. Your example query will just *fail* without them (assuming that a sane `name` field will contain some letters). 2. escaping, when applicable, WILL "protect from people altering your query logic in malicious ways" for sure. 3. I dunno what lays under ADODB prepared statements but I suspect it's the same old good escaping (at least for sake of compatibility). 4. None of your good practice examples demonstrate a case when a field name have to be added dynamically. – Your Common Sense Nov 07 '11 at 07:39
  • Don't take it as offense please. I am concentrated on the matter, not appearance and if you feel some phrasing being impolite, it is not intentionally. – Your Common Sense Nov 07 '11 at 07:49
  • 1
    I do not mind constructive criticisms :) 1. You are correct,my mistake, should be ID not NAME. 2. Well regarding escaping, these are really not my conclusions http://www.slideshare.net/billkarwin/sql-injection-myths-and-fallacies look slide 47. 3. I do not know too, but I always thought that prepared queries are executed slightly differently from regular queries. 4. For inserting field names dynamically I would use a "whitelist" approach as suggested by the same video 'SQL Injection Myths & Fallacies', perhaps I should include an example. – GEMI Nov 07 '11 at 08:25
  • There are native prepared statements and emulated ones. PDO is using emulation by default, I suspect the same behavior for ADODB. – Your Common Sense Nov 07 '11 at 08:32
  • Would you be so kind as to suggest some reading materials or videos? Also do you have any thoughts how to improve my answer? – GEMI Nov 07 '11 at 08:36
  • well, I still fail to see on the slide 47 anything relevant to the "It helps, but surely this is not the best practice." statement. can you explain? what is the reason for escaping+quoting for being not a best practice? or you referring to the escaping alone? it is poorly placed then as it makes ambiguous statement. – Your Common Sense Nov 07 '11 at 08:38
  • I dunno the answer quality. I just had a feeling that you missed some nuances. May be I was wrong. It's all okay than :) – Your Common Sense Nov 07 '11 at 08:40
  • Well I might have to rephrase that. What I really mean is that if there is a possibility that if I misuse my escaping and it allows some malicious logic to slip in, then it is better to use parametrized queries. Where such possibility does not exist, at least for simple queries with no dynamic column or table values. Oh regarding the slides it is actually 47 through 50 :) Sorry. – GEMI Nov 07 '11 at 08:48
  • Well, in general I agree with your conclusions, though I'd phrase it different. A general rule of safety may be formulated as "As long as you are adding dynamic parts to the query through *placeholders* (and these placeholders being processed properly), you are safe.". Placeholder not necessarily may represent a [native] prepared statement, but can be a substitution for the field name or escaped value as well. – Your Common Sense Nov 07 '11 at 08:59