sql security issue. prevent terminating the SQL statement prematurely ?

Hi,

Do the update in a transaction and roll it back on error. What type of component is temp_sql?

Regards, Geo

Use a parameterized query. "Update Table1 set customerID=:customerID where ID=5"
With a parameterized query, the parameters are just considered to be values, not part of the query.

The risk is simple: SQL code injection. If someone enters '";delete from table1;select * from table1 "t' in hidden_cust_id.text then your query becomes:

UPDATE table1 SET CustomerID= "";delete from table1;select * from table1 "t" WHERE Id=5

And you immediately notice that three valid queries will be executed. One query that sets all customer ID's to 0. A second query that deletes all records, and a third where just records are selected. Although the last one might fail. And because it fails, the transaction would be rolled back.

But if none of the inserted queries fail, they could do quite a lot of damage... For example, killing all your tables or granting access to the wrong people. A security risk.

My advise to avoid these risks is simple: ALWAYS use parameterized queries...

hmm. that would be ok for the next prj, but now i have lot of code that works other way

any transform function that will replace all appearances of " and ; maybe?

your thoughts?

thanks again

try to convert the field into integer. this way you can be sure that you have value

var
  CustID   : Integer;
begin
  try
    CustID := StrToInt(hidden_cust_id.text);
  except
    // if error ...
    MessageDlg('Wrong Customer ID', mtError, [mbOK], 0);
    // exit from this routine
    Exit;
  end;
  //  ... your SQL execution below
  with datamodule1.temp_sql do
    begin
      sql.clear;
      sql.add('UPDATE table1 SET ');
      sql.add('   CustomerID= "' + hidden_cust_id.text + '" ');
      sql.add('WHERE Id=5';');
      execsql;
    end;  

still remains text fields, for example fields with customer names etc....

Well, you can make the form validation in separate method. Just like this is done on website registrations forms. You can use the VBScript ot JavaScript to validate your form (if they are complex) or write some small procedure (if you need to check 5-6 controls)...

Ok,
but what you mean by:

>>Which is pretty harmless again. But it doesn't make you 100% invulnerable.

what else can user do to corupt db data? any related links to this issue?

Users can insert invalid data, of course. :-)

It depends a bit on the SQL database you're using but some other characters might cause minor problems too. But they are more likely to crash your application than to crash the database.
But a hacker could try to discover the connection that you're using to connect to the database. (Especially username and password.) With this information, a hacker could get full access to your database. And of course there could be some other flaws. Don't worry too much, though. Your code will be a lot more save. But no system is 100% secure anyway.

The nastiest thing a hacker could try is code injection. A DLL gets injected in the process space of your application and is used to search for any database access. It could then interfere with it. It's just that it requires an experienced hacker with access to your system and in such a case, he has control over it anyway... Basically, how secure does the application has to be?

Basically, what I suggested (doubling all ' and " characters) should be save enough in 99.999% of all situations.

It is vulnerable because, if I know you wil add " I can write taht text:

" ; delete * from table; update table set field="

Then, when you add "", it becomes valid SQL but again will delete all you table!

No, because you'd replace ALL "'s by two "'s. Thus "" becomes """" and '' becomes ''''.

using mysql server v4.0.2.0
ssl encrypted connection
security level should be at high level,
this app will be distributed to limited number of people.

But workshp, if i write where a="" and you change single " with "", then you get this trash where a="""" with 4 ", that will not work... changin " with double "" is a bad solution... using params is the only thing that will let you sleep well at night.

What about takin ";" away from text? If you detect one, then stop executing as multiple SQL statement can be chained.

Sergio, you're not replacing the "'s in the query but in the values you're including to the query. thus:

      sql.clear;
      sql.add('UPDATE table1 SET ');
      sql.add('   CustomerID= "' + StringReplace(hidden_cust_id.text, '"', '""', [rfReplaceAll]) + '" '); // Double quotes
      sql.add('WHERE Id=5';');
or
      sql.clear;
      sql.add('UPDATE table1 SET ');
      sql.add('   CustomerID= ''' + StringReplace(hidden_cust_id.text, ''', '''', [rfReplaceAll]) + ''' '); // Single quotes
      sql.add('WHERE Id=5';');

Thus, a user enters "" as customer ID. Apparantly he wants two double quotes. To insert a double-quote in a double-quoted string, you must add it twice in general, so the SQL parser recognises it as a double quote. Btw. If you use " in your SQL around string constants, you don't have to double the single quotes, and vice versa.

If you write: where a = ""
Then that will be part of the query, which you won't have to change. All you change are the parameters. Besides, if the user wants to insert a " or ; to the text, he should just be allowed to do so.

Of course, using parameterized queries is better. But this works quite well too. Remember, the SQL parser will replace all "" by a single " and won't consider it the end of the string. If you would enter: " ; delete * from table; update table set field="
Then the database will use that value exactly as you entered it, with a " at the beginning and the end.

There's one risk that I can come up with now and that's when a user manages to enter a CRLF to the input string. In that case, the string constant will be divided over two lines and the system might break again. But it will just break things, not damage data. Besides, most editboxes won't allow you to insert a CRLF without doing some complex hacking trick.
Furthermore, since Delphi controls don't support unicode by default, the list of dangerous characters are limited to only a few. The Carriage Return, the LineFeed and either the single or double quote. Oh, and the Null character, of course. But Delphi controls won't easily accept them.