November 17, 2011 at 3:29 pm
Fantastic comment Steve...
"This isn't good for your brand as a developer. If you don't know what SQL Injection is, you shouldn't be developing software. If you don't know how to code to avoid it, you shouldn't be hired by anyone to build software. If you can't write a stored procedure around a query or built a parameterized call to a database engine, you need to learn how or find another career."
November 17, 2011 at 3:33 pm
As long as a company has not been "burnt" , that is has not lost a significant number of $$ by an injection attack, then the cost savings of not modifying code becomes significant. Now to reverse the situation, where the most important project for all IT personnel is to protect / thwart / stop an injection attack, lets make it a law that for each bit of information divulged due to an attack the company has to pay a significant amount of $$ to each and every individual who has any data reveled, even something as insignificant as age / marital status / sex. Then the bookkeepers will find the necessary funds to allow for old code to be reviewed, modified or rewritten, and a major objective will be added to all code reviews .. i.e., testing for an injection attack.
November 17, 2011 at 3:52 pm
Revenant (11/17/2011)
Eric M Russell (11/17/2011)
Dave62 (11/17/2011)
Jack Corbett (11/17/2011)
It's sad but I've seen code that is ripe for SQL Injection, commented it shouldn't be allowed and that if it was my decision it wouldn't be, yet the code gets pushed out because the "project must get done".:crazy: In the time it takes to write the comment/excuse the embedded SQL could have been copied and pasted to a strored procedure.
If management is so whacked and out of it, would they even notice if you just make the SQL Injection fix, and bundled it another change order? Especially during the development phaze. There are some simple fixes that can be implmented within the stored procedure to scrub input parameters, etc. without even touching the web application stuff.
Of course, it also helps to insure that the application account is running with least required privilages. Obviously the application account doesn't need sysadmin or dbo role membership, so that avoids the possibility of stuff like dropping tables, querying schema, or selecting from any table.
Usually it is a good idea not to allow users access to the tables, only to views and sprocs.
Exactly my point, and that goes for the web application account as well. Just add the account to the [PUBLIC] role and then GRANT EXEC on whatever minimum set of stored procedeures it needs. On SQL Server 2005/2008, that configuration is hard to exploit.
"Do not seek to follow in the footsteps of the wise. Instead, seek what they sought." - Matsuo Basho
November 18, 2011 at 6:37 am
Dave62 (11/17/2011)
Jack Corbett (11/17/2011)
It's sad but I've seen code that is ripe for SQL Injection, commented it shouldn't be allowed and that if it was my decision it wouldn't be, yet the code gets pushed out because the "project must get done".:crazy: In the time it takes to write the comment/excuse the embedded SQL could have been copied and pasted to a strored procedure.
I agree. Of course both of us would have to have more details about the application in order to understand how to fix the core problem. Based on what I know about the application, I don't know that SP's are even an option. I'm not even sure you can do parameterized calls. Let's just say it's an ugly app from the database perspective at least.
Jack Corbett
Consultant - Straight Path Solutions
Check out these links on how to get faster and more accurate answers:
Forum Etiquette: How to post data/code on a forum to get the best help
Need an Answer? Actually, No ... You Need a Question
November 18, 2011 at 6:41 am
Eric M Russell (11/17/2011)
Dave62 (11/17/2011)
Jack Corbett (11/17/2011)
It's sad but I've seen code that is ripe for SQL Injection, commented it shouldn't be allowed and that if it was my decision it wouldn't be, yet the code gets pushed out because the "project must get done".:crazy: In the time it takes to write the comment/excuse the embedded SQL could have been copied and pasted to a strored procedure.
If management is so whacked and out of it, would they even notice if you just make the SQL Injection fix, and bundled it another change order? Especially during the development phaze. There are some simple fixes that can be implmented within the stored procedure to scrub input parameters, etc. without even touching the web application stuff.
Of course, it also helps to insure that the application account is running with least required privilages. Obviously the application account doesn't need sysadmin or dbo role membership, so that avoids the possibility of stuff like dropping tables, querying schema, or selecting from any table.
If only it were so simple. I'm not really involved in this application. I happened to hear them talking about a SQL Server error that they were getting, so, being the SQL Server guy, I stuck my head in to help figure out the SQL Server error. That's when I saw the crappy code. Oh, and it isn't a stored procedure, it's inline dynamically generated SQL. Generated incorrectly in this case as well.
I'm pretty sure the application account doesn't have sysadmin, but probably does have dbo. As I mentioned in a previous post, it is all-around an ugly app from the SQL Server perspective.
Jack Corbett
Consultant - Straight Path Solutions
Check out these links on how to get faster and more accurate answers:
Forum Etiquette: How to post data/code on a forum to get the best help
Need an Answer? Actually, No ... You Need a Question
November 18, 2011 at 7:41 am
Jack Corbett (11/18/2011)
Eric M Russell (11/17/2011)
Dave62 (11/17/2011)
Jack Corbett (11/17/2011)
It's sad but I've seen code that is ripe for SQL Injection, commented it shouldn't be allowed and that if it was my decision it wouldn't be, yet the code gets pushed out because the "project must get done".:crazy: In the time it takes to write the comment/excuse the embedded SQL could have been copied and pasted to a strored procedure.
If management is so whacked and out of it, would they even notice if you just make the SQL Injection fix, and bundled it another change order? Especially during the development phaze. There are some simple fixes that can be implmented within the stored procedure to scrub input parameters, etc. without even touching the web application stuff.
Of course, it also helps to insure that the application account is running with least required privilages. Obviously the application account doesn't need sysadmin or dbo role membership, so that avoids the possibility of stuff like dropping tables, querying schema, or selecting from any table.
If only it were so simple. I'm not really involved in this application. I happened to hear them talking about a SQL Server error that they were getting, so, being the SQL Server guy, I stuck my head in to help figure out the SQL Server error. That's when I saw the crappy code. Oh, and it isn't a stored procedure, it's inline dynamically generated SQL. Generated incorrectly in this case as well.
I'm pretty sure the application account doesn't have sysadmin, but probably does have dbo. As I mentioned in a previous post, it is all-around an ugly app from the SQL Server perspective.
No public facing web app needs to drop tables or perform other Database Owner operations. By denying the web app account view schema permission, a hacker attempting to exploit the account would no way to discover what objects are available, which would handicap them in a big way. Also, most apps that perform dymanic sql typically need d/u/i permission on a handful of tables and select permission on the remainder. In some cases, an app only needs select and insert permission. That would at least prevent them from going back and updating or deleting historical records.
"Do not seek to follow in the footsteps of the wise. Instead, seek what they sought." - Matsuo Basho
November 21, 2011 at 4:13 pm
it isn't a stored procedure, it's inline dynamically generated SQL.
But wouldn't they be using sp_executesql if they're using polished dog-turds dynamically generated sql?
November 21, 2011 at 8:36 pm
niall.baird (11/21/2011)
it isn't a stored procedure, it's inline dynamically generated SQL.
But wouldn't they be using sp_executesql if they're using
polished dog-turdsdynamically generated sql?
Heh... every one knows you can't polish a dog-turd. As you said before, you have to roll it in glitter. 😀
--Jeff Moden
November 21, 2011 at 8:39 pm
From the Article:
There's no excuse for companies not making a complete review of older code and updating it to avoid unvalidated input or passing through queries that could be hacked.
Why sure there is... 😉 It's the same ol' crap excuse that (it seems) the majority of developers and their pointy-haired managers use... "We had to meet the schedule to hit the release date."
Obviously, this article hit a real sensitive spot with me and rather than rant for an hour about the real underlying problem that it addresses, I'll just say that the following quote, also from the article, is absolutely spot on!
If you don't know what SQL Injection is, you shouldn't be developing software. If you don't know how to code to avoid it, you shouldn't be hired by anyone to build software. If you can't write a stored procedure around a query or built a parameterized call to a database engine, you need to learn how or find another career.
--Jeff Moden
November 21, 2011 at 8:54 pm
Obviously, this article hit a real sensitive spot with me and rather than rant for an hour about the real underlying problem that it addresses, I'll just say that the following quote, also from the article, is absolutely spot on!
If you don't know what SQL Injection is, you shouldn't be developing software. If you don't know how to code to avoid it, you shouldn't be hired by anyone to build software. If you can't write a stored procedure around a query or built a parameterized call to a database engine, you need to learn how or find another career.
Where is my +1 for this?
That being said, (and now o/t), any chance of getting the google+ *+*?
Viewing 10 posts - 16 through 24 (of 24 total)
You must be logged in to reply to this topic. Login to reply