More SQL Injection

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

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

    If everything seems to be going well, you have obviously overlooked something.

    Ron

    Please help us, help you -before posting a question please read[/url]
    Before posting a performance problem please read[/url]

  • 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

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

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

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

  • 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-turds dynamically 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


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

    Change is inevitable... Change for the better is not.


    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)

  • 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


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

    Change is inevitable... Change for the better is not.


    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)

  • 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