Stopping embedded sql

[Jason Kwok]

Hi,

I have a simple search form with just a text field and a submit button. I'm using the MVC pattern and I want to be able to stop users from embedding SQL into the search field and possible harming the database.

For example, they may enter something like 'jones; DELETE * FROM Users;' into the field and submit.

The controller just retrieves the string from the request object and passes it to my model, that executes an SQL command like so: 'SELECT * FROM Teachers WHERE name = "+name (where name is the parameter passed from controller to model.

However, it would also attach the ;DELETE * FROM Users; and effectively delete everything in the users table, if such a table exists in the first place.

How can I stop this? Would setting permissions on the database to read only be enough and catch the exception? Or is there something else that can be done?

Thanks,
Jason

[Bear Bibeault]

SQL injection is real threat that needs to be taken seriously. It's not, however, a servlet issue. So this has been moved to the JDBC forum.

[Bear Bibeault]

Step 1:

Creating Statements by concatenating parameter values into the SQL string is madness:

that executes an SQL command like so: 'SELECT * FROM Teachers WHERE name = "+name (where name is the parameter passed from controller to model.

Use a PreparedStatement providing the parameter as, well, a parameter!
[ March 26, 2007: Message edited by: Bear Bibeault ]

[Tim Holloway]

In other words:

final static String SQL_STMT =
  "SELECT a, b, c FROM teachers WHERE name=?";
 
PreparedStatement pstmt =
  conn.prepareStatement(SQL_STMT);
pstmt.setString(1, request.getParameter("teacherName"));
 
ResultSet rs = pstmt.executeQuery();
while ( rs.next() ) {
  ....
}
pstmt.close();

I'm docking Bear 5 marks for using a "SELECT *". Those things break when someone adds a new column to the table.

BTW, I've not been allowed to use Java for about 3 months now - just .Net and Python. So if the above code even compiles, I'll be amazed.

Other offenses it commits are:

1. No try/catches to avoid resource leakage if something blows.

2. No test for malformed input. If the teacher name parameter was omitted from the request, it's going to look for items whose name was null. Probably not what you wanted.

However, it should be safe from SQL injection attacks.

Given a choice, I'd be using Spring to wrap for the SQL error handling, and use of an ORM framework would allow me a lot more power - and probably even more safety, but if you're not granted such liberties, a PreparedStatement will at least make things much safer than roll-your-own SQL.
[ March 26, 2007: Message edited by: Tim Holloway ]

[Jason Kwok]

Hi Tim,

I'm familiar with the prepared statement as you suggested, but I'm still rather new to Java and unsure if it will the stop the sql injection.

Using your example, if I enter 'jones; DELETE * FROM Teachers;' into the form and submit... won't the code then create the following:

SELECT a, b, c FROM Teachers WHERE name=jones; DELETE * FROM Teachers;

Or will something else happen?

Thank you so much for your help.

Jason

[vu lee]

Would setting permissions on the database to read only be enough and catch the exception? Or is there something else that can be done? [/QUOTEs]

Setting the roles to restrict who could delete, modify, and insert records would be a good step to guard toward the backend. You can always validate the front end to detect whether the search criteria is valid and not violating the constraints such as throwing exception if the search criteria matches certain pattern-- containing word such as delete, update, insert, etc. The earlier you catch, the less work needs to be done on the back end.

[Herman Schelti]

hi Jason,

If you use PreparedStatements you will get:
SELECT a, b, c FROM Teachers WHERE name='jones; DELETE * FROM Teachers'

It (probably) won't find any teachers, but it won't hurt either.

OK?

Herman

[Jason Kwok]

Hi Herman,

I didn't mean the single quotes would be included... I put those there to separate the string so as not to confuse any readers. I guess I failed on that part

I think something like this will occur: SELECT a, b, c FROM Teachers WHERE name=jones; DELETE * FROM Teachers;

when entering: jones; DELETE * FROM Teachers;

Jason

[Paul Clapham]

Originally posted by Jason Kwok:
I think something like this will occur: SELECT a, b, c FROM Teachers WHERE name=jones; DELETE * FROM Teachers;

Well, if we're voting then Herman and I outnumber you. Herman is correct. But if you don't believe either of us you could try it for yourself.


[ March 27, 2007: Message edited by: Paul Clapham ]

[Jason Kwok]

Sorry I didn't mean to be rude, I just don't understand how the prepared statement will remove part of the parameter like that. I'm just too curious for my own good.

I'll test it out, I'm sure it works well.

Thanks to all who replied,
Jason

[Paul Clapham]

No, I didn't mean you were rude, exactly. Looks more like you misunderstood: the PreparedStatement doesn't remove anything, it just forces data to be treated as a parameter rather than as part of the SQL. Let me suggest that you set up a row in the Teacher table where the "name" column contains

jones; DELETE * FROM Teachers;

You should be able to find that row by using a PreparedStatement and entering

jones; DELETE * FROM Teachers;

into the form.

[Jason Kwok]

Hi Paul,

After reading your last post, I finally understand what everyone in this thread was trying to tell me. I now see how the prepared statement will work to solve my problem. It makes perfect sense.

Thank you very much,
Jason