Review Your Code

  • Comments posted to this topic are about the item Review Your Code

  • Surely the amnesty cutoff date should be whenever this was posted? 🙂

    http://xkcd.com/327/

  • 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.

  • 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:

    - 😀

  • 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
    Performance Problems
    Common date/time routines
    Understanding and Using APPLY Part 1 & Part 2

  • 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.

  • 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.

  • 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.

  • 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?

  • 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.

  • TGwinn (8/12/2013)


    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?

    No, I didn't check the date. Denny posted a note just the other day about this, and I thought it was new. My mistake.

  • If we want to see the back of sql injection harpooning the developers is probably the wrong way to go about it. Yes they should be using parametized queries and yes you could fire them for concatenating values into their sql strings but the chances are the next developer you hire is going to do the same damn thing... even if you specifically ask it as an interview question. There will always be bad developers and some of them are going to sneak in under the radar.

    What I'd like to see is this prevented at the development language level. I'd like to see is an abstraction that sits above the level of ADO etc. and builds my sql for me in a guaranteed safe fashion. It should pull in the schema and allow me to select the various sql operations etc. Basically something similar to the query builder that comes in management studio or access. Of course, those query builders are truly horrible which is why nobody uses them so this abstraction would have to be GOOD. I'm not sure what it would look like, whether it would be graphical or textual but it would have to be easy to use, powerful and flexible. It would also need to keep up with new additions to the various flavours of sql. It would be no mean feat to create such a tool but until it exists I will continue to write my own sql strings and, as long as I do that, I am more than capable of idiocy.

  • There are OTC solutions that will read your code and scripts and warn you of potential security problems. All of the SQL Injections are identified and you can change as needed. Problem is that these solutions are very costly and take a specialized skill set. And they can add weeks to the release cycle depending on what is found in the QA/QC and Security check. Many companies will not hold a production release of software for a Security evaluation. The idea is to get to market as quickly as possible and to make money!!! And this will continue for as long as Marketing is allowed to control the release dates and versions.

    Not all gray hairs are Dinosaurs!

  • I agree with what I think the general consensus is: the time when this was acceptable as a flaw due to lack of knowledge and education has passed.

    Gaz

    -- Stop your grinnin' and drop your linen...they're everywhere!!!

  • Gary Varga (8/22/2013)


    I agree with what I think the general consensus is: the time when this was acceptable as a flaw due to lack of knowledge and education has passed.

    Yes, that time passed long ago.

    back in the summer of 1988, when an SQL Injection attack managed to result in one and a half million web pages on US and UK government sites being sufficiently damaged that they began to spread malware, it was already long past the time when people should have stopped writing stuff that allowed this to happen, but there were idiots claiming that preventing injection was very difficult to do. I commented on it then (not on SQL Server Central):

    (8/13/2008)


    I find it appalling that our government has so many sites that suffer this problem, in fact appalling isn't strong enough it is outright disgraceful. It's equally disgraceful that there are those here who are commenting to the effect that avoiding SQL Injection isn't easy, when in fact it is pretty trivial.

    It seems to me to be amazingly easy to design and produce web applications, even using oldish technology like MS ASP with ADO (and SQL Server 2000) that are free from any possibility of succumbing to web injection attacks, and permitting injection seems actually to require a extreme lack of awareness of simple principles of design.

    The situation has not changed: it's just as easy with ASP.NET as it was with ASP, and with SQLS 2012 as it was with SQLS 2000. But people still want to construct queries by concatenating user supplied strings in the front end, which in my opinion is definitely be an offence for which a web developer should be fired (if the employee protection rules / laws permit it).

    Tom

Viewing 15 posts - 1 through 15 (of 16 total)

You must be logged in to reply to this topic. Login to reply