114 Comments
- Zenham, on 08/25/2008, -4/+66You know, it's not very hard to parse passed data for SQL injection. It's even easier to encode/escape passed data before storing to "defang" it.
The developers who leave that sort of hole open aren't likely to have secure applications for dozens of other reasons; I'm not sure having another layer of abstraction will help in the long run, here.
If your shirt is on fire, putting on another shirt over the burning one isn't going to help you in the long run. - Phocion55, on 08/26/2008, -4/+54Obligatory XKCD reference - http://xkcd.com/327/
- Jforsyth89, on 08/26/2008, -1/+42; DROP TABLE digg_users ;
Am I doing it right? - inactive, on 08/26/2008, -1/+33completely worthless. teach people to write good code (prepared statements, taint checking etc) rather than write a program to allow bad programmers to continue on with writing crap code!
also web firewalls are meant for this stuff, try mod security - thisguy47, on 08/26/2008, -0/+23Is it bad if I know exactly which xkcd that is without clicking it?
- Number23, on 08/26/2008, -2/+201. Users are evil, they are your enemy. Never trust them
2. Treat all user input as suspect
3. Scrubb user input for special characters
4. Validate user input for type, length, format and expected values
5. Really, avoid at all cost, using user input in a WHERE clause and the like - thecheatah, on 08/26/2008, -1/+19JUST BIND UR VARIABLES. STOP LIVING THE FREAKING PAST. SQL INJECTION IS A SOLVED PROBLEM.
- inactive, on 08/26/2008, -3/+20More like DROP TABLE MrBabyMans_Submissions
And you need a single quote at the beginning - NathanielJ, on 08/26/2008, -0/+14The following might be a bit more appropriate, since if MrBabyMan has his own table then there's something seriously wrong.
'; DELETE FROM Submissions WHERE User='MrBabyMan - RyeBrye, on 08/26/2008, -2/+16A SQL firewall? that's cute.
- thecheatah, on 08/26/2008, -2/+15Just bind ur damn variables. Make it practice. THAT'S ALL YOU NEED TO DO. DONT NEED TO ESCAPE QUOTES OR ANYTHING.
(unless someone can tell me otherwise!) - snowblindnz, on 08/26/2008, -5/+18People coding websites at a database level should be shot. You should be using a class or db wrapper that supports escaping / has it built in, and you should never use a user account with drop/delete access on a live server unless they need it.
- tylermenezes, on 08/26/2008, -0/+10Okay, here's my solution. I run a special build of MySQL. It reads queries BACKWARDS. Thus, SELECT * FROM users; becomes ;sresu MORF * TCELES
That'll teach those script kiddies. - Haplo, on 08/26/2008, -0/+8I am somewhat amazed that a lot of people here actually suggest to escape your data. It's wrong.
use binding. It's that simple.
If you are escaping data and joining strings together to make a query, you shouldn't write code for a web application, period. - rocketman42, on 08/26/2008, -1/+9No offense, but it sounds like you're part of the problem. "use SSL certs between the web and SQL server and encrypt all the transactions" Really? How exactly would that protect against sql injection? MS Sql Server is actually a pretty good database. If you're using that, I have to assume you are using other MS development tools. Even going back to old ASP, it was easy to prevent these attacks by using ADO Command objects. .Net has made it even easier. Perhaps you should take a class or something before trying to make fun of people over something you clearly don't understand.
- inactive, on 08/26/2008, -0/+8I'm not sure why there are all the "learn to code stupid" comments. Sure your average and above coder doesn't need this, but a well-configured set-up of this software sounds like a useful addition for a shared hosting company: (a) it protects users from their own stupidity and (b) it's a nice feature they can 'sell' in a highly competitive market.
Phillip. - sfrench, on 08/26/2008, -0/+7+1 for everybody that said "prepared/bound statements"
- tj111, on 08/26/2008, -0/+7'; DELETE FROM digg_users WHERE name LIKE '%KevinRose%';
- rayofpith, on 08/26/2008, -0/+6Two words: prepared statement
- sesstreets, on 08/26/2008, -0/+5http://www.0x000000.com/?i=558
Much easier way. - HeavyWave, on 08/26/2008, -0/+5I'm sorry but your "pretty" SQL is not supported by Oracle.
- Haplo, on 08/26/2008, -0/+5"Preventing SQL injections takes a single-line replace command to check for quotes."
It's even more simple: use binding. Your "solution" is a good example of a significant part of the problem: people who think they are smart enough to clean up the data before they glue it together. It's wrong, don't do it, even though PHP has been somewhat promoting this for some time. - rocketman42, on 08/26/2008, -0/+5Wow, if you're a developer and you 1) don't know what binding means and 2) can't use Google to find out, then you really need to find a new career.
- NathanielJ, on 08/26/2008, -1/+6Yeah, it pretty much is. As long as you make sure that numeric values are actually numeric (which you should be doing anyways) and that quotations aren't floating around unchecked, you're pretty much set. Data validation should be done anyways and is a much better solution than adding in a package to make it so you don't have to validate.
- Rikkochet, on 08/26/2008, -0/+5From a business standpoint, it seems to make a hell of a lot more sense to fix the problem that exists rather than take everything back to the drawing board because it's become a pig over multiple major revisions.
I mean, I've been in your shoes frequently but looking back it would have been silly to recode something that doesn't need that much maintenance. - grumpyrain, on 08/26/2008, -1/+5The fact that it was a Microsoft server has no relevance to the discussion. SQL Injection is not a flaw in the DBMS, it is your application code that allowed a malicious user to execute of an arbitrary queries.
Use paramatised queries. This creates a distinction for the dbms between the query logic and the values applying for that logic. When I see (in the article) the query delete from foo where ?=?, I just cringe. Why would you let someone arbitrarily select the field to perform the comparison on? Why not just punch yourself in the face. In every environment I have seen, parameter inputs are automatically sanitised. If you dynamically build up SQL text, then there is a fair chance it can be exploited. - ieatpizza, on 08/26/2008, -0/+4It's cute...... BUT IT'S WRONG
- inactive, on 08/26/2008, -2/+6Ha,Ha,Ha...He said injection.
- Chalks777, on 08/26/2008, -0/+4oh man, you'd be surprised. I hang out on a developer forum, and the code that some people post is simply horrendous. Also, you get people who read a "how to build a website in 10 days" tutorial, and they have no idea how to protect against that kind of thing. They don't even know it's an issue.
- pearcewg, on 08/26/2008, -0/+4Were you using a Microsoft .NET application layer for this database? If so, there are database layer objects you can get for .NET that will prevent SQL Injection, and make class objects for each database SP. This of course requires usage of SPs...if you are using embedded SQL, I would recommend using objects (since you are using an enterprise class database anyway).
- scubajim, on 08/26/2008, -1/+5You are absolutely correct. Too often developers try to build some sort of filtering mechanism. They haven't a clue. Just use bind variables. It is more perfomarnt, more scalable, and you don't have ot worry about sql injection. And you actually write less code.
- Haplo, on 08/26/2008, -0/+43) = wrong, if you use binding there is NO NEED. Scrubbing user input is part of the problem: people constantly forget about things that should be scrubbed.
Do not glue strings together to make a query. Use prepare and binding. Then you don't have to worry about missing a special case. That way, you don't have to worry about 5 either. - Haplo, on 08/26/2008, -0/+4"SQL protection should be accomplished through input validation and escaping."
You're doing it wrong. SQL protection should be done by using a library that let you prepare statements and use binding. If you are escaping data in your code to prevent SQL injection, you are already on the path to fail. - Haplo, on 08/26/2008, -0/+4You should *never* have to parse your data for SQL injection. Use a library that supports binding. A large part of SQL injections happen because people forgot to parse it right. Moreover, it's silly to write the same stuff each and every time. So: use a library that supports binding.
- creole, on 08/26/2008, -1/+5Ahem...
It's not just ColdFusion that's involved. One of the more recent attacks started off against ASP (ASPROX). PHP is just as vulnerable as well. It really has nothing to do with the language, and everything to do with the coding habits of the programming team.
In fact ColdFusion, with it's built in construct cfqueryparam, can be better prepared against SQLi attacks than other web-based languages:
http://cfquickdocs.com/cf8/#cfqueryparam - scubajim, on 08/26/2008, -0/+4No Moron you should be using bind variables.
- Haplo, on 08/26/2008, -1/+4You're doing it wrong.
You should not escape your input, but use binding (Perl example):
my $sth = $dbh->prepare( 'SELECT * FROM table WHERE ID=?' );
$sth->execute( $id ); # no need to escape $id
$sth->finish;
It's that simple. Please read perldoc DBI again. - boojoy, on 08/26/2008, -0/+3This is a lot of work for something that is at its core pretty simple. If you need help use a good framework with some built in database abstraction classes.
- Haplo, on 08/26/2008, -1/+4Again wrong answer: if you're escaping/filtering, you're on the road to utter fail. use binding.
- deadmoo, on 08/26/2008, -0/+3PHP magic quotes don't provide enough protection. What if the variable I am injecting is an integer?
- greeniemeani, on 08/26/2008, -1/+4Regex or escaping characters or anything else is doing it wrong, except in the case that the language you are using to query the database doesn't support binding of parameters.
Just bind the ***** parameters...
And btw, PHP5 has bound parameters, so stop that mysql_real_escape_string *****. - fuzzlog, on 08/26/2008, -0/+2Writing SQL query statements is so 2001. We use Hibernate with criteria. No more injection. Anything the user inputs is validated against type and value. And all statements are similar to prepared statements so that very specific queries are used in all cases.
- Terr01, on 08/26/2008, -0/+2Down that path lies madness... and reinventing the wheel.
First you escape the single-quotes (perhaps with ' ) Then you need to escape the escape symbols that were present in the original text, but not so that they undo your hard work... etc. etc. - Lunarbunny, on 08/26/2008, -0/+2http://www.w3schools.com/sql/
- DanAtkinson, on 08/26/2008, -0/+2I don't like this idea one bit. It's encouraging programmers to write lax code. They should never rely on a SQL firewall, and should instead be learning how to write code which is secure on its own, without a firewall.
The number of 'seasoned' developers who are blown away by prepared statements tells me that there's something very wrong. This GreenSQL only encourages that. - DCstewieG, on 08/26/2008, -0/+2Use Hibernate.
- expert01, on 08/26/2008, -0/+2In the movie, they can pretend it's one of those lame 90's hacker movies, and they have to run around everywhere with guns and ***** to stop the hackers, except the hackers' only weaknesses are sex.
Oh yeah, the hackers can just type a few lines to get into the website. - charlietuna, on 08/26/2008, -0/+2excellent!
- senfo, on 08/26/2008, -0/+2> Doesn't work too well with .NET/MS SQL Server though.
That's what NHibernate is for. - scubajim, on 08/26/2008, -0/+2If you are too f'ing lazy to read the manuals I can't do it for you. EVERY RDBMS worth its salt strongly recommends using bind variables in the programmers guide. (some environments call them host variables, Java calls them prepared statements) RTFM.
-
Show 51 - 100 of 116 discussions

What is Digg?