Click here to monitor SSC
SQLServerCentral is supported by Redgate
 
Log in  ::  Register  ::  Not logged in
 
 
 


Review Your Code


Review Your Code

Author
Message
Steve Jones
Steve Jones
SSC-Dedicated
SSC-Dedicated (35K reputation)SSC-Dedicated (35K reputation)SSC-Dedicated (35K reputation)SSC-Dedicated (35K reputation)SSC-Dedicated (35K reputation)SSC-Dedicated (35K reputation)SSC-Dedicated (35K reputation)SSC-Dedicated (35K reputation)

Group: Administrators
Points: 35976 Visits: 18726
Comments posted to this topic are about the item Review Your Code

Follow me on Twitter: @way0utwest
Forum Etiquette: How to post data/code on a forum to get the best help
My Blog: www.voiceofthedba.com
paul.knibbs
paul.knibbs
SSCommitted
SSCommitted (2K reputation)SSCommitted (2K reputation)SSCommitted (2K reputation)SSCommitted (2K reputation)SSCommitted (2K reputation)SSCommitted (2K reputation)SSCommitted (2K reputation)SSCommitted (2K reputation)

Group: General Forum Members
Points: 1972 Visits: 6211
Surely the amnesty cutoff date should be whenever this was posted? :-)

http://xkcd.com/327/
OCTom
OCTom
SSCrazy
SSCrazy (2.8K reputation)SSCrazy (2.8K reputation)SSCrazy (2.8K reputation)SSCrazy (2.8K reputation)SSCrazy (2.8K reputation)SSCrazy (2.8K reputation)SSCrazy (2.8K reputation)SSCrazy (2.8K reputation)

Group: General Forum Members
Points: 2813 Visits: 4152
I may need some educating here. But, is the article Denny Cherry referenced really discussing a SQL injection? It was injecting malicious Javascript to a web page. It seems to me that's different than a SQL injection.
Andrew Kernodle
Andrew Kernodle
SSChasing Mays
SSChasing Mays (610 reputation)SSChasing Mays (610 reputation)SSChasing Mays (610 reputation)SSChasing Mays (610 reputation)SSChasing Mays (610 reputation)SSChasing Mays (610 reputation)SSChasing Mays (610 reputation)SSChasing Mays (610 reputation)

Group: General Forum Members
Points: 610 Visits: 8135
Ugh, this rings far too true at present. The company I work for currently uses third-party software, and after examining the system, I've found that its search procedures are all very SQL-injectable by running some test injections against it. Worse yet, the search procedures aren't stored procedures, they're just SQL strings concatenated together in the ASP.NET front-end.

Not sure if I can rewrite these monstrosities without breaking the service agreement with the third-party company, and they've said they don't intend to fix the vulnerability. If I had a say in it, the whole software company would be "fired" from our usage, and their program replaced w00t

- :-D
WayneS
WayneS
SSCertifiable
SSCertifiable (6.2K reputation)SSCertifiable (6.2K reputation)SSCertifiable (6.2K reputation)SSCertifiable (6.2K reputation)SSCertifiable (6.2K reputation)SSCertifiable (6.2K reputation)SSCertifiable (6.2K reputation)SSCertifiable (6.2K reputation)

Group: General Forum Members
Points: 6222 Visits: 10398
IMO, the amnesty cutoff date should be 2003. And not just for SQL Injection - there's all kinds of vulnerabilities in code that have been exposed (another example that should have died a long time ago is cross-site scripting), methods to do it safely and properly have been implemented, and the dang developers are either completely incompetent or too lazy to do it right. Fire them all, and get competent programmers that will actually work with the DBA to do it correctly.

But it boils down to the companies. If you are going to have an emphasis on hiring the cheapest person, you get what you pay for. If you aren't going to have regular training on how to do things correctly (including newly discovered things outside of your company), you deserve what's coming to you. If you sweep it under the rug... shame on you. Where is the code review for this code that's being written with all of these vulnerabilities? That usually comes down to a resource availability to do it - something that the company controls and not the developer. There are programs that will test your code for vulnerabilities - not using these is a company decision. This is a topic that you just can't take a shortcut with.

From a developer standpoint, there's no excuse for implementing such shoddy code.
From a business standpoint, there's no excuse for allowing it. And the onus is on the business to ensure that it is done properly. If your business has an environment where this is allowed to persist, there's no incentive to ever change from "we've always done it this way". And your developers never will.

And if your business sells this product, your business should be held liable for every individual instance of these security preventative measures not being followed, with a minimum fine a $500,000 (USD) per occurrance. Unfortunately, businesses usually won't change until it becomes less expensive to do it right the first time, and the only way this will happen is by steep fines when they don't.

Wayne
Microsoft Certified Master: SQL Server 2008
Author - SQL Server T-SQL Recipes
If you can't explain to another person how the code that you're copying from the internet works, then DON'T USE IT on a production system! After all, you will be the one supporting it!
Links: For better assistance in answering your questions, How to ask a question, Performance Problems, Common date/time routines,
CROSS-TABS and PIVOT tables Part 1 & Part 2, Using APPLY Part 1 & Part 2, Splitting Delimited Strings

brdudley
brdudley
SSCommitted
SSCommitted (1.5K reputation)SSCommitted (1.5K reputation)SSCommitted (1.5K reputation)SSCommitted (1.5K reputation)SSCommitted (1.5K reputation)SSCommitted (1.5K reputation)SSCommitted (1.5K reputation)SSCommitted (1.5K reputation)

Group: General Forum Members
Points: 1533 Visits: 1737
Set the amnesty date in 2003 and you'd have to fire some Microsoft programmers. I was shocked to see sample code on the architecture and practices site with concatenated queries much later than that.

Worse still, recent version of SSIS have decreased the opportunities to add parameters to queries on sources, opting for expressions. The chance of a SQL Injection in SSIS is very rare, but it sets the wrong expectations and habits.

It is a correct observation that this problem stems from low cost which is further fueled by the low barrier to entry. As a profession, we have done a bad job of setting standards and communicating the value of those standards.

I expect this will get much worse before it gets any better. New web companies emerge every day and each is an opportunity for bad code to expose private information. As far as fining the offending companies...the big guys have their license agreements written to exclude damages incurred from their bugs. I don't expect that to change either.
Steve Jones
Steve Jones
SSC-Dedicated
SSC-Dedicated (35K reputation)SSC-Dedicated (35K reputation)SSC-Dedicated (35K reputation)SSC-Dedicated (35K reputation)SSC-Dedicated (35K reputation)SSC-Dedicated (35K reputation)SSC-Dedicated (35K reputation)SSC-Dedicated (35K reputation)

Group: Administrators
Points: 35976 Visits: 18726
OCTom (8/12/2013)
I may need some educating here. But, is the article Denny Cherry referenced really discussing a SQL injection? It was injecting malicious Javascript to a web page. It seems to me that's different than a SQL injection.



The JS is injected into the database. It's spread the next time a dynamic version of a page is rendered. At least, that's my understanding and how I've seen things spread before.

Follow me on Twitter: @way0utwest
Forum Etiquette: How to post data/code on a forum to get the best help
My Blog: www.voiceofthedba.com
Steve Jones
Steve Jones
SSC-Dedicated
SSC-Dedicated (35K reputation)SSC-Dedicated (35K reputation)SSC-Dedicated (35K reputation)SSC-Dedicated (35K reputation)SSC-Dedicated (35K reputation)SSC-Dedicated (35K reputation)SSC-Dedicated (35K reputation)SSC-Dedicated (35K reputation)

Group: Administrators
Points: 35976 Visits: 18726
brdudley (8/12/2013)
Set the amnesty date in 2003 and you'd have to fire some Microsoft programmers. I was shocked to see sample code on the architecture and practices site with concatenated queries much later than that.

Worse still, recent version of SSIS have decreased the opportunities to add parameters to queries on sources, opting for expressions. The chance of a SQL Injection in SSIS is very rare, but it sets the wrong expectations and habits.


I completely agree, but I'd also put this same burden on bloggers/ speakers. Stop showing sa/blank, or joke about sa/test as an easy combination. Stop showing simple passwords and bad code.

We are also to blame in pushing out bad code.

Not that MS has any excuse. They should practice what they preach, even if it means slightly delayed products or more architecture.

Follow me on Twitter: @way0utwest
Forum Etiquette: How to post data/code on a forum to get the best help
My Blog: www.voiceofthedba.com
TGwinn
TGwinn
SSC Journeyman
SSC Journeyman (79 reputation)SSC Journeyman (79 reputation)SSC Journeyman (79 reputation)SSC Journeyman (79 reputation)SSC Journeyman (79 reputation)SSC Journeyman (79 reputation)SSC Journeyman (79 reputation)SSC Journeyman (79 reputation)

Group: General Forum Members
Points: 79 Visits: 454
Steve says today (8/12/2013) in the editorial "I wasn't surprised when a piece from Denny Cherry appeared recently"

Um, the Dennis Cherry post says "This entry was posted on Monday, October 24th, 2011 at 2:36 pm".

The code snippet linked in the Cherry post was posted 10/12/2011.

The IT World article linked in the Cherry post was posted 10/24/2011.

Is this perchance a reprinted editorial from 2011?
OCTom
OCTom
SSCrazy
SSCrazy (2.8K reputation)SSCrazy (2.8K reputation)SSCrazy (2.8K reputation)SSCrazy (2.8K reputation)SSCrazy (2.8K reputation)SSCrazy (2.8K reputation)SSCrazy (2.8K reputation)SSCrazy (2.8K reputation)

Group: General Forum Members
Points: 2813 Visits: 4152
Steve Jones - SSC Editor (8/12/2013)
OCTom (8/12/2013)
I may need some educating here. But, is the article Denny Cherry referenced really discussing a SQL injection? It was injecting malicious Javascript to a web page. It seems to me that's different than a SQL injection.



The JS is injected into the database. It's spread the next time a dynamic version of a page is rendered. At least, that's my understanding and how I've seen things spread before.


Thanks Steve for the explanation.
Go


Permissions

You can't post new topics.
You can't post topic replies.
You can't post new polls.
You can't post replies to polls.
You can't edit your own topics.
You can't delete your own topics.
You can't edit other topics.
You can't delete other topics.
You can't edit your own posts.
You can't edit other posts.
You can't delete your own posts.
You can't delete other posts.
You can't post events.
You can't edit your own events.
You can't edit other events.
You can't delete your own events.
You can't delete other events.
You can't send private messages.
You can't send emails.
You can read topics.
You can't vote in polls.
You can't upload attachments.
You can download attachments.
You can't post HTML code.
You can't edit HTML code.
You can't post IFCode.
You can't post JavaScript.
You can post emoticons.
You can't post or upload images.

Select a forum

































































































































































SQLServerCentral


Search