Solved

sql security issue. prevent terminating the SQL statement prematurely ?

Posted on 2004-09-16
14
251 Views
Last Modified: 2010-04-05

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
Comment
Question by:zastava101
  • 5
  • 4
  • 2
  • +2
14 Comments
 
LVL 17

Expert Comment

by:geobul
ID: 12073409
Hi,

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

Regards, Geo
0
 
LVL 17

Expert Comment

by:Wim ten Brink
ID: 12073605
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
 

Author Comment

by:zastava101
ID: 12074087
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
 
LVL 12

Expert Comment

by:Ivanov_G
ID: 12074098
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
 

Author Comment

by:zastava101
ID: 12074213
still remains text fields, for example fields with customer names etc....
0
 
LVL 12

Expert Comment

by:Ivanov_G
ID: 12074980
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
 
LVL 17

Accepted Solution

by:
Wim ten Brink earned 150 total points
ID: 12075821
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
IT, Stop Being Called Into Every Meeting

Highfive is so simple that setting up every meeting room takes just minutes and every employee will be able to start or join a call from any room with ease. Never be called into a meeting just to get it started again. This is how video conferencing should work!

 

Author Comment

by:zastava101
ID: 12082411
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
 
LVL 17

Expert Comment

by:Wim ten Brink
ID: 12082928
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
 
LVL 6

Expert Comment

by:Sergio_Hdez
ID: 12083015
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
 
LVL 17

Expert Comment

by:Wim ten Brink
ID: 12089144
No, because you'd replace ALL "'s by two "'s. Thus "" becomes """" and '' becomes ''''.
0
 

Author Comment

by:zastava101
ID: 12091333
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
 
LVL 6

Expert Comment

by:Sergio_Hdez
ID: 12109718
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
 
LVL 17

Expert Comment

by:Wim ten Brink
ID: 12110615
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

Featured Post

How your wiki can always stay up-to-date

Quip doubles as a “living” wiki and a project management tool that evolves with your organization. As you finish projects in Quip, the work remains, easily accessible to all team members, new and old.
- Increase transparency
- Onboard new hires faster
- Access from mobile/offline

Join & Write a Comment

Introduction The parallel port is a very commonly known port, it was widely used to connect a printer to the PC, if you look at the back of your computer, for those who don't have newer computers, there will be a port with 25 pins and a small print…
Have you ever had your Delphi form/application just hanging while waiting for data to load? This is the article to read if you want to learn some things about adding threads for data loading in the background. First, I'll setup a general applica…
When you create an app prototype with Adobe XD, you can insert system screens -- sharing or Control Center, for example -- with just a few clicks. This video shows you how. You can take the full course on Experts Exchange at http://bit.ly/XDcourse.
This video explains how to create simple products associated to Magento configurable product and offers fast way of their generation with Store Manager for Magento tool.

707 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

17 Experts available now in Live!

Get 1:1 Help Now