Improve company productivity with a Business Account.Sign Up

x
  • Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 264
  • Last Modified:

sql security issue. prevent terminating the SQL statement prematurely ?


im using following like code in my program:

  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;  

but i was told there may be security issues if i didnt check what user has entered into hidden_cust_id.text. Is there some way to terminate current sql command and to write other one using this input parameter (hidden_cust_id.text)? Any way to prevent that? any checking(filtering) routine?

thanks in advance.
0
zastava101
Asked:
zastava101
  • 5
  • 4
  • 2
  • +2
1 Solution
 
geobulCommented:
Hi,

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

Regards, Geo
0
 
Wim ten BrinkSelf-employed developerCommented:
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...
0
 
zastava101Author Commented:
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
0
Free Tool: ZipGrep

ZipGrep is a utility that can list and search zip (.war, .ear, .jar, etc) archives for text patterns, without the need to extract the archive's contents.

One of a set of tools we're offering as a way to say thank you for being a part of the community.

 
Ivanov_GCommented:
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;  
0
 
zastava101Author Commented:
still remains text fields, for example fields with customer names etc....
0
 
Ivanov_GCommented:
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)...
0
 
Wim ten BrinkSelf-employed developerCommented:
Well, there is a simple solution by replacing all ' and " that the user adds to the input with two ' or ". Something StringReplace could easily do for you. In that case, the dangerous query I showed would change into:

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

Which is pretty harmless again. But it doesn't make you 100% invulnerable.
0
 
zastava101Author Commented:
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?
0
 
Wim ten BrinkSelf-employed developerCommented:
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.
0
 
Sergio_HdezCommented:
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!
0
 
Wim ten BrinkSelf-employed developerCommented:
No, because you'd replace ALL "'s by two "'s. Thus "" becomes """" and '' becomes ''''.
0
 
zastava101Author Commented:
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.
0
 
Sergio_HdezCommented:
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.

0
 
Wim ten BrinkSelf-employed developerCommented:
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.
0
Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.

Join & Write a Comment

Featured Post

Free Tool: IP Lookup

Get more info about an IP address or domain name, such as organization, abuse contacts and geolocation.

One of a set of tools we are providing to everyone as a way of saying thank you for being a part of the community.

  • 5
  • 4
  • 2
  • +2
Tackle projects and never again get stuck behind a technical roadblock.
Join Now