- Feature Articles
- CodeSOD
- Error'd
-
Forums
-
Other Articles
- Random Article
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
Um, yeah, my thought exactly... Espcially considering SQL server 2000 came out in, wait for it, 2000 and .Net 1.0 came out in 2002. So the likelyhood of any .Net app, new or old, using anything SQL 7 or before is very low. I've seen it, but again unlikely. Also, stored procedures existed in SQL 7 (not sure about versions before that though).
Admin
How old were you when 2000 rolled around? Do the letters Y2K mean anything to you? It was short for "The Year 2000". This was significant because programmers from the 1970's did not anticipate their software would be around until the turn of the century, leading to a problem because of assumptions that only two digits were needed to represent the year.
Moral of the story: yes, software can be and often is old.
Admin
My, aren't we a pretentious prick today?
Any idiot can change the scope of the conversation at any point to match their argument, but I thought we were staying focused. My mistake...
In any case, IF you were referring to this particular WTF, it's written in .Net... Also, yes, .Net can target pre SQL 7 (or most any DB for that matter including oracle, MySQL, terradata, etc.), but if you took a second to look, you would notice the provider is SQL server.
Moral of the story: you don't know everything, and acting like a wanker makes it that much more obvious.
Admin
Who knew?
Admin
I have one. Use to develop for a document search product that used SQL, tied with its own additions, in order to perform read only. No binding or parameters allowed so string concatenation was the only way. So yes SQL injection was available but anything but a select was ignored.
Admin
Maybe you THINK you know what you're talking about, but I have enough evidence here to show the likelihood that I should explain to you why you don't.
This code is .net (out 2002), and has been written (at authorship) via Ole to SQL Server, which is capable of command parametrization since version 7 (out 1998).
My only assumption here is that this code has not been written for a version of SQL Server prior to version 7, and this assumption has a way bigger probability to be correct than the assumption that the author of this code knew or cared about parametrized command but couldn't implement it.
Admin
Admin
Admin
Let me put it another way--have you ever worked on code with a long lifetime? This sort of duty is called "maintenance," something that occurs after the initial development effort has concluded. In this context, the fact that it is .Net code (although THAT is not really indicated here--it could very easily be C++) is immaterial. As technology evolves and your software migrates to many platforms, it is very often the case that you have to modify the code to perform the same operation using a slightly different context. This migration falls under the larger umbrella called "refactoring", which has the same basic goal: make the product work like it did before you changed what exists "under the covers."
It is idiotic when you migrate code to start chasing rabbit trails and make the software do what it did not do originally--this should be part of a subsequent operation generally called "enhancement" or, if it is a serious enough defect, "bug fixing." If you try to do this as part of the "refactoring" effort, you have no baseline and an enormous potential to lose important data. I've met plenty of junior developers that fail to understand this fundamental concept.
Admin
FTFY, and I'm so sorry to hear that...
Admin
I'd estimate to have asked around 10-15 people that question. This is not for a DB admin role, but it is for a role which requires someone to be reasonably comfortable with sql queries, and where only people indicating as such on their resumes are brought in for an interview. (here's the twist.. turns out they are usually lying).
Admin
Admin
FTFY
Admin
And your implication is that no software was written before the 90's?
Can you explain what you see in this picture? [image]
Admin
Admin
Admin
Admin
SQL injection would be redundant. What I don't get is how he missed the fact that his code returned a value vastly larger than the dataset?
Admin
Or at least I believe that is how it works in Oracle. I can't speak for SQL Server and others.
Admin
Admin
Ah, so you did change the topic. Oooh, can I play too?
You moron, obviously XSS is a security concern too! And JavaScript has been around since the mid '90s.'
Clearly you didn't hear the voices in my head the same way I did...
Admin
Admin
FTFY. Did you mean CURRENT_TIMESTAMP?
Also: the SQL Server Provider has been around for ages. No excuse for using such a stupid connection string.
Admin
You're just kidding, right? You're not a retard?
Admin
Oh, he's a retard alright, but this time he's kidding.
Admin
In SQL Server, if you issue the following statements from an application and use string concatenation instead of parameterization:
EXEC GetEmployee 1234 EXEC GetEmployee 2345
... you will get plenty of cache benefits, but no injection prevention.
If you issue SELECTs, but properly parameterize them, you will get injection protection, and you might get statement caching through a mechanism called "ad-hoc batch caching". Ad-hoc batch caching does auto-parameterization, so two calls can have different text, but still use the same execution plan.
Admin
Admin
Finally, we are getting somewhere... near interesting.
Admin
It's easy to forget that these are separate issues when you always see it done either one way (prepared statements, properly parameterized) or the other (SQL string with params concatenated in; ready for injection attacks!).
Admin
Admin
To be fair I've never argued that stored procedures should be used to protect against SQL injection. My opinion of stored procedures is really no different than for any other form of subroutine, and that's the only reason I've ever advocated using them.
But I do see where you're coming from; the solution (stored procs, and even prepared statements) is unrelated to the problem (SQL injection), and most people fail to realize this. Thanks for clarifying.
Admin
It could be worse; At least isn't counting the rows recursively!
Admin
Okay, so junior developer here. I just learned SQL ( from a book ) and now I have to learn web programming to maintain the code to the DB. I understand most of the bad things here, ( though not all ) but what is correct way to prevent sql injection, so my code doesn't end up here ?
Admin
There also is: opening a new connection on each call!
Admin
Admin
It's not a WTF, and is actually recommended practice if database connection pooling is available.
Admin
Just to join in on the "sanitizing inputs" free-for-all:
My general philosophy is: Sanitize inputs at point of input. Then you can rely on their being valid throughout the system. If the field is entered on a screen by the user, we validate when processing the input and before saving to the database or using in any calculations. If the field is received from another system, we validate at the time we get the message or whatever. Etc.
Yes, you COULD write all your code checking the validity of every parameter for every function. But then you have six bazillion validity checks. It seems to me that this violates one of the most basic principles of good programming: Don't Repeat Yourself. This: (a) Wastes CPU cycles validating the same field over and over. (b) Makes the program more difficult to maintain. If the validity test changes -- maybe a field that used to be limited to 8 characters is now allowed to be 9 -- we have a bazillion places to change instead of one. (c) Makes the code harder to read. Every function has so-many lines of extra code to do this validity testing, which clutters up the logic. (d) Makes the user interface less friendly, as instead of alerting the user to bad data at the time he tries to enter it, we presumably accept it, and then fail later when we try to use it. The user who gets the error message may not be the user who entered the bad data, or even if it's the same person he may have entered the bad data months ago and has no easy way to determine what the correct data is. (I suppose we could be validating at input time and also validating when we try to use it, in which case this objection would not be applicable.)
Yes, this philosophy means that if a foolish programmer fails to validate an input and writes it to the database, functions could fail all over the place. But if we have bugs in our code, surely the solution is to fix the bug, not to write other code to circumvent the bad effects of the bug.
Admin
As for DRY, nobody writes SQL sanitation code; it's built into every sane library on the planet. All you have to do is parameterize your calls (which you should be doing anyways because it often provides other benefits).
Admin
Validating certainly should be done at the point of input, but I'd suggest that sanitizing wait until it's actually necessary. You may need the value unchanged for prior operations, but more importantly, there is more than one way to sanitize depending on how you use the value (HTML encoded, URL encoded, or whatever).
Admin
One word: canonicalization. The same data can be in differenent encodings, different targets, etc. The problem with only validating at point of entry is that there is a possibility of the data being used elsewhere.
For example: you have a field that searches a SQL database table by a users first name. Now in the UI you sanitize the data looking for 'DROP', or 'CREATE', or '--', etc. Now lets say you start using the same data to query LDAP, or you're going to look into an XML file, etc. Now your 'sanitization' targeted to a DB is useless as a malicious user can now perform LDAP or XML/xpath injection attacks...
Here is a link, as they word it much better than I can...
Admin
He is iterating through the entire table (very slow) instead of using the sql 'count()' function (very fast).
Admin
They make such pearls to convince users to buy more expensive hardware to run their shit. Welcome to the industry!
Admin
Over and over again "the daily wtf" suprises me. One should think there must be something on an programmers mind. If I read such code I just can think, "It surely has nothing to do with programming"....
Admin
how about this:
int GetLeadCount(SqlDb voDb, int vnHomeID, ReviewStatusEnum veReviewStatus) { string lsSql = "SELECT COUNT(*)" + " FROM LeadsTab" + " WHERE HomeID = " + vnHomeID + " AND ReviewStatus = " + (int) veReviewStatus;
}
when using only int's, you needn't escape the sql-string parts - no danger but faster to type.
Admin
He nievely returned all the data from the database using the select * and then preoceeded to loop though the datareader to count the rows.
When all he really needed to do was 'select count(*)' which would have returned the count for him.
Admin
CAPTCHA: distineo
sp_executesql('SELECT DISTINEO ' & columns & from & join & where & group & having & order)
Admin
SQL Injection tops the Homeland Security list of website problems:
http://www.nytimes.com/2011/06/28/technology/28secure.html?_r=1
Admin
He could have inserted his records into a DataGridView then retrieved the RowCount property... :)
Admin
I think the point was that it is perfectly possible to write NON-SQL-injectable code via string concatenation, assuming you use an appropriate escaping function. No one stated that it is OK to write injectable code, but have fun with your strawman.
For instance if you are using conacenation to build a SQL statement in PL/PGSQL, you would use quote_literal or quite_ident as appropriate. This code is not vulnerable to injection.
If you were doing the same thing in PHP (for postgres), you would use pg_escape_string. There are genuine occasions where you might want to do this. An example might be where the database is remote from the client and the latency between the two is very high (dwarfing query execution time), preparing the query can actually slow things down because of the initial round trip to the DB doing the prepare added to the round trip time of the query when executed. When you write apps in the third-world with third-world infrastructure, reality sometimes gets in the way of your convenient "*NEVER".
Admin
BUT if you are using database connection pooling, you should not be constructing your connection string on the fly. Connection pooling depends on recognising that a new connection matches an existing one, ie has a functionally identical connection string.