Win a copy of Design for the Mind this week in the Design forum!
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Stopping embedded sql

 
Jason Kwok
Ranch Hand
Posts: 126
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Author and ninkuma
Marshal
Pie
Posts: 64700
86
IntelliJ IDE Java jQuery Mac Mac OS X
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Author and ninkuma
Marshal
Pie
Posts: 64700
86
IntelliJ IDE Java jQuery Mac Mac OS X
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Saloon Keeper
Pie
Posts: 18094
48
Android Eclipse IDE Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
In other words:



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
Ranch Hand
Posts: 126
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Ranch Hand
Posts: 206
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Ranch Hand
Posts: 387
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Ranch Hand
Posts: 126
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Sheriff
Pie
Posts: 20958
31
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Ranch Hand
Posts: 126
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Sheriff
Pie
Posts: 20958
31
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Ranch Hand
Posts: 126
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic