The Daily WTF: Curious Perversions in Information Technology

Grimoire · 2007-03-23 Reply Admin

JCM:
Waitaminute. I have had good uses for goto in C# code.
My code looks something like this:
runTest:

try
{
   this.RunTest();
}
catch(Exception e)
{
   if(e.Message.IndexOf("SA Application") != -1)
   {
      goto runTest;
   }
   else
   {
      throw e;
   }
}
Better way to solve this problem?

bool runTest = true;

while(runTest)
{
    runTest = false;
    try
    {
        RunTest();
    }
    catch(Exception e)
    {
        if(e.Message.IndexOf("SA Application") != -1)
        {
            runTest = true;
        }
        else
        {
            throw e;
        }
    }
}

2007-03-23 Reply Admin

Wouldn't that be an infinite loop? How about

bool succeeded = false; int attempts = 0;

while ((! succeeded) && (attempts < MAX_ATTEMPTS)) { try { RunTest(); succeeded = true; } catch (StupidThirdPartyExcepion stpe) { attempts++; } }

2007-03-23 Reply Admin

Lurker:
Wouldn't that be an infinite loop?

Only if it fails every single time with the same error message. It was stated that after such an error, an immediate retry will work.

2007-03-23 Reply Admin

savar:
Ytram:
Off-topic: Why do people announce their CAPTCHAs?

Because they're nerds..

Nerds? I guess...I have a stronger word in mind.

Seriously, I fail to see why a person would post a comment like "CAPTCHA: Xevious. I've never played Xevious, actually.." Who the hell cares?

Grimoire · 2007-03-23 Reply Admin

Lurker:
Wouldn't that be an infinite loop? How about
bool succeeded = false; int attempts = 0;

while ((! succeeded) && (attempts < MAX_ATTEMPTS)) { try { RunTest(); succeeded = true; } catch (StupidThirdPartyExcepion stpe) { attempts++; } }

I simply reworked the original to not use gotos. The algorithm is the same, which says "call RunTest until it works". But yes, to truly fix it, I would catch the specifically thrown exception (assuming that RunTest doesn't actually throw Exception) and have a maximum number of attempts.

2007-03-23 Reply Admin

Xandax:
Heh - even my company have made "SQL-languaged" banner for job applicants. Think it is a rather common strategy to separate yourself from the normal crowd.

Am I the first to smell the irony in this post?

Well, if it's "rather common", how can you "separate yourself" from the "normal crowd?", given that normal is common?

2007-03-23 Reply Admin

Ryan S:
There's another ad (I see it mostly on slashdot) just like this for the same business I believe, except it uses PHP:
It flashes a few times between

<?php format='The %2s contains %1d orders'; printf(format, num, location); ?>
and
<?php $format='The %2$s contains %1$d orders'; printf($format, $num, $location); ?>

This really annoys me as a PHP developer because they both have a few blatant, even-my-PHB-knows-this mistakes (vars need a $, undefined constants/variables "num" and "location", '%2$s' and '%1$d' are syntactically invalid...)

(captcha: I like how the word "smile" actually looks somewhat like a smile)

Actually, '%2$s' and '%1$d' are valid placeholders in printf. So much for the 'blatant, even-my-PHB-knows-this' mistakes.

regards

fg

PSWorx · 2007-03-23 Reply Admin

Pap:
Is this actually live somewhere? And does it actually allow SQL injection? If not, then then this WTF is completely pointless. All the advertisement needs to do is ADVERTISE, not be Turing complete or anything like that.
If they were looking for website developers and made an advertisement that said
I need a new job! 
Would you rag on it for missing the DOCTYPE declaration and not validating?

I agree completely. If the script behind that ad really would be prone to SQL injection, yeah, it would be a WTF. (Although I'd like to see the other application then that makes use of the rows with JOB_SATISFACTION != "HIGH") But pointing and laughing at that add because - lo and behold - a fictional query MIGHT be prone to SQL incection IF it happened to become real somehow is just lame. You don't complain about format taggers that miss out the opening <sarcasm> tag either, do you?

Julian · 2007-03-23 Reply Admin

I've never been able to avoid a code elegance question. So how about this:

do {
	try {
		RunTest();
		break;
	} catch (Exception ex) {
		if (ex.Message.IndexOf("SA Application") < 0) {
			throw;
		}
	}
} while(true);

The do/while syntax feels a little more natural to me in this, it cleanly throws the original exception and it doesn't use a state variable. If you needed to count attempts, the following is quite entertaining:

public void TryRunTest(int attempts) {
	try {
		RunTest();
		return;
	} catch (Exception ex) {
		if (ex.Message.IndexOf("SA Application") < 0 || attempts <= 0) {
			throw;
		}
		attempts--;
	}
	TryRunTest(attempts);
}

Which employs tail-recursion to stop filling up the stack.

Of course, there are good reasons there is a goto in the language (try breaking out of a nested loop sometime). But to be honest the original formulation was pretty clear. It didn't have any serious problems. Hence, I claim it's good code (even if I've just written two variants on it...) :)

2007-03-23 Reply Admin

panzi:
If this is actually how you do job searches on their site, they may want to add a disclaimer; Please do not enter the following in either field: ";DELETE FROM JOBS;SELECT * FROM JOBS WHERE "1" = "1

Is this a joke or does it really work??

Yes, has anyone proved that this doesn't use SQL parameters, or submit to something other than SQL?

Little Thunder · 2007-03-23 Reply Admin

PSWorx:
I agree completely. If the script behind that ad really would be prone to SQL injection, yeah, it would be a WTF. (Although I'd like to see the other application then that makes use of the rows with JOB_SATISFACTION != "HIGH") But pointing and laughing at that add because - lo and behold - a fictional query MIGHT be prone to SQL incection IF it happened to become real somehow is just lame. You don't complain about format taggers that miss out the opening <sarcasm> tag either, do you?

As I see it, the question of whether the website actually allows SQL injection attacks is not the point.

It's a reasonable assumption that the job site was created to attract good programmers, and this particular ad was created to attract good SQL coders.

The ad expresses the coder's desire to find a programming position with high job satisfaction, and it does so with a little bit of humor. The humor comes from displaying the search criteria framed as a simple SQL statement. The humor should make the ad and the website attractive to good SQL coders.

The WTF is that the simple SQL framing will make good SQL coders recoil in horror because it appears to allow dangerous SQL injection attacks. I.e., the ad works against itself because it repels the people it wants to attract.

Little Thunder

2007-03-23 Reply Admin

I think the webmaster was paid money from Dice to make this WTF. I think it's the only reasonable explanation.

2007-03-23 Reply Admin

JCM:
Waitaminute. I have had good uses for goto in C# code.
Let's say I'm talking to some instrument like a spectrum analyzer through a less-than-reliable driver supplied by the instrument vendor, whose name is something like Agile Ant. Lets say that this driver sometimes mysteriously barfs, throwing an Exception with the message "SA Application not installed." Let's also say that the driver behaves normally again with an immediate retry. Let's also say that the Agile Ant people appear to have no desire to fix this problem. My code looks something like this:
(gross)
Better way to solve this problem?

Yes - write a subroutine. They came up with this concept in the 1960s (or was it the 1950s?) so people wouldn't have to continue writing GOTOs.

public void SomeMethod()
{
    bool success = false;
    do
    {
        success = RunSafeTest();
    } while (!success);
}

private bool RunSafeTest()
{
    try
    {
        RunFlakeyTest();
        return true;
    }
    // Catch the specific exception, unless it actually throws "Exception"
    catch (SpecificException se)
    {
        if (IsStupidException(se))
            return false;
        throw se;
    }
}

private static bool IsStupidException(SpecificException se)
{
    return se.Message.Contains("SA Application");
}

This might have taken an extra 30 seconds to write, but think how much easier it is to understand. If you hadn't given us the background information about this Agilent bug and just posted that code without context, I'd be asking you, "What the hell is that goto for?"

And if someone else discovers in the future that there are actually 10 totally different StupidExceptions, they know exactly where to look to add support for them. Try to imagine if IsStupidException were to become a complicated test with several conditionals and loops - in your original, you'd now need either a bunch of "gotos" or a bunch of flags and the whole thing would lose coherence pretty fast!

Yours looks OK now because there's just one goto, dealing with just one edge-case condition, but there's no guarantee that it will stay like that. Please, have a heart for the poor schmucks who might have to maintain your code some day.

2007-03-23 Reply Admin

It's a job board, the point of the ad is that IT people can go there, and find a job with high job satisfaction. A knee jerk reaction OMGWTF SQL INJECTION LOLZ!!1!!1!eleven!! is just stupid, and frankly I wouldn't want to employ someone who took that attitude, so maybe it was their intention to turn you off.

Since half the commenters didn't seem to know what SQL Injection was before this post, maybe they should hold their criticism?

themagni · 2007-03-23 Reply Admin

Aaron:
Yes - write a subroutine. They came up with this concept in the 1960s (or was it the 1950s?) so people wouldn't have to continue writing GOTOs.

GOTO is a valid instruction on most (all?) architectures.

The End.

2007-03-23 Reply Admin

Ytram:
Off-topic: Why do people announce their CAPTCHAs?

Me, i do it because of frustration with CAPTCHAs. some sites have some pretty bad ones, and it's hard for me to get them 100% (I guess that makes me a robot). It's fun to have something that's different, and readable, and relevant. I guess i also do it if it reminds me of an older article - I'll always announce a Brillant! CAPTCHA.

CAPTCHA: burned well, i guess no brillant for me today

2007-03-23 Reply Admin

CAPTCHA: muhahaha

I was feeling a little nerdy today...

aquanight · 2007-03-23 Reply Admin

Julian:
I've never been able to avoid a code elegance question. So how about this:
do {
	try {
		RunTest();
		break;
	} catch (Exception ex) {
		if (ex.Message.IndexOf("SA Application") < 0) {
			throw;
		}
	}
} while(true);

Of course, I'm personally annoyed at the use of looking for a hardcoded exception message, instead of the proper way - make the desired "retry it" exception a subclass, and throw it instead of hardcoding a string in Exception.

while(true) {
  try {
    RunTest();
    break;
  } catch (SAApplicationException e) {
    continue;
  }
}

Not that hard to make an exception class. Seriously. At minimum, all it takes is

public class SAApplicationException : Exception {}

One line. And since this isn't like java where you're limited to 1 public toplevel class per file (and if it was, you could just slap on a static and throw it inside the toplevel class), you could just shove it at the bottom (or top) of the file with RunTest (I personally wouldn't make a whole file for 1 line, that's just wasteful). It's only when you want fancy things (like a default exception message) does it need more than that (still cheesy simple: just add public SAApplicationException() : base("default") {} public SAApplicationException(String blah) : base(blah) {}).

Pap · 2007-03-23 Reply Admin

Little Thunder:
The WTF is that the simple SQL framing will make good SQL coders recoil in horror because it appears to allow dangerous SQL injection attacks. I.e., the ad works against itself because it repels the people it wants to attract.
Little Thunder

There's nothing wrong with the SQL as long as input validation and/or escaping has been done in the application. OBVIOUSLY, nobody is going to dump an entire application codebase into the advertisement. It's one line of SQL out of many lines of application code.

2007-03-23 Reply Admin

I'm sorry, I'm going to have to disagree with the WTF'edness of this.

The assumption here is that the SQL statement is part of a web form. I don't think the ad was trying to convey that. It's obviously an ad, so it's really just using a well-known SQL statement to convey the fact that you can find good jobs in a city of your choice. It's a metaphor - not a real web form.

To those saying "This would make a real programmer's blood curdle" - that's only under the assumption that user input is being placed directly in the SQL statement. I think the intention was to imply that dice.com's purpose is much the same as that SQL statement. The text prompts are merely a shorthand for "whatever type of job you want", "whatever city you want your job in".

An ad that could be misinterpreted? Maybe. But this is a poor choice for a front-paged WTF on what's supposed to be a tech-related blog.

2007-03-23 Reply Admin

Who's calling us nerds for posting captchas?

You leave me no choice. I must strike you down with my plastic lightsaber. Oh, and I will be wearing my Darth Vader suit, so I'm afraid my THAC0 will be most impressive.

captcha: quantum entanglement. (I didnt like the one they provided, so I provided my own.

chrismcb · 2007-03-23 Reply Admin

Ytram:
Yeah, this was a pretty weak Error'd article. Just because some marketing guys thought this would be cute does not mean it's a SQL injection vulnerability.
Off-topic: Why do people announce their CAPTCHAs?

What I don't understand is, why regular users even see a CAPTCHA...

2007-03-23 Reply Admin

Funny thing is how the "SQL injections" have become the rage now.

I remember many years back (early 80s) when I was an IT student (or DP as it was then called) that we had a brand new Data General MV6000 mini, on which students were not allowed to access the CLI command line.

Our student menu, however, was a CLI script and it didn't me too long to figure out that a menu option of ;prompt/k;[ created a necessary "unmatched [" message and dropped me to command line.

I guess some tricks are as old as the hills.

Jeff S · 2007-03-23 Reply Admin

CaptainObvious:
Jeff S:
to protect against injection, all you have to do is use parameterized commands. You should *never* concatenate anything together with a SQL statement and execute it, unless the language/framework/database you are using doesn't support parameters. Of course, in that case, you probably have bigger issues to worry about.

Never is such a horrible word to use. I concat strings for SQL execution all the time, well, I use System.String.Format, actually, but same effect:

System.String.Format("SELECT * FROM Foo WHERE Bar={0}", SQLExcape(szUserInput));

Now, concatenating raw user input to SQL without escaping it, thats just stupid... Although I do have that one application I use for executing arbitrary SQL, that is by its nature an acceptable use of unescaped SQL, but there is no concatenation involved then, so it doesn't qualify.

I stand by it. Never. Ever.

see: http://weblogs.sqlteam.com/jeffs/archive/2006/07/21/10728.aspx

When you don't use parameters, you are not passing VALUES to the SQL server -- you never pass anything other than STRINGS. Which means that all data to be passed must first be formatted and converted to strings, and then sql must convert the data from a string back to the correct datatype. This means that suddenly date and numeric and boolean (bit) formats at the client versus the server become very important, since EVERY VALUE that gets passed to the database must be formatted as string. i.e., your data goes from the native types at the client to a strings which must be converted back to a native type by the server. Does this make sense? Why would you do this if you didn't have to?

Using parameters is quick, short, easy, safe, and correct.
It is actually NOT lazy to "avoid" parameters, it is actually easier to use parameters and experience much less of a headache, for all the reasons I demonstrate in my article. Using String.Format() is easier than direct concatenation, but things STILL must always be converted and delimited and escaped as strings, and you loose the benefit of quickly looking at your sql code to see @UserID and @AccountNumber sprinkled throughout instead of {0} and {1}.

Of course, using stored procs is even better. but we won't go there .... after all, that is debatable, I suppose, and it certainly has been quite a bit over the years. But whether or not to use parameters simply isn't debatable.

Be smart. Do it right. Do it quick. Do it easily. Write clearer code ... and use parameters. There are very few "always" and "nevers" when it comes to programming, but this is one of them.

2007-03-23 Reply Admin

Not that hard to make an exception class. Seriously. At minimum, all it takes is One line.

How, exactly, do you propose he do this to someone else's code? If he could modify it, don't you think he'd fix the root problem, rather than just neatening up the exception it shouldn't be throwing in the first place?

--

The 'in reply to' links are blue and underlined; if you click them it shows you the comment the person you're reading was reading when he wrote what you're reading. Keep clicking them, and you'll have the context for the whole discussion.

tendrel · 2007-03-23 Reply Admin

hmmm...I am a bit autistic and take a lot of stuff really literally, but I got the irony and the humor...

I also laughed really hard at all the comments. Especially the rewriting of the code with the goto, and recognition by at least one person that the original was OK considering the situation and the restrictions in that situation.

I mean, I can totally relate to saying that you need to consider who comes after, and make it super clear and proper. However, if the situation really is static, meaning no more variations later, then the original was clear, assuming that there are some comments somewhere.

the openly condescending and hostile comments are just as funny, but really just such a downer in other ways.

overall, this was an interesting one. thanks.

2007-03-24 Reply Admin

Actually, you cannot generally drop tables with this kind of SQL injection. The query is usually constructed with string concatenation, then executed. If you were to try to embed a DELETE FROM or DROP TABLE statement, you would encounter an error -- you are trying to execute two queries through an interface intent on parsing one query.

The real danger with SQL injections lies in the ability to select records, not in the ability to damage them.

Example (php syntax):

Base query: "select count(1) from users where uname = '" . $uname . "' and passwd = '" . $password . "';"

this is a very common (and poor) authentication query. The trick to SQL injection is thus...

in uname field: ' or 'x' = 'x' ' in passwd field: ' or 'x' = 'x' '

Using these entries, you authenticate to the first user/password without even having a valid username!

2007-03-24 Reply Admin

CaptainObvious:
Jeff S:
to protect against injection, all you have to do is use parameterized commands. You should *never* concatenate anything together with a SQL statement and execute it, unless the language/framework/database you are using doesn't support parameters. Of course, in that case, you probably have bigger issues to worry about.

Never is such a horrible word to use.

But in this case, it's absolutely appropriate. NEVER EVER use dynamic SQL techniques to generate static SQL. Not even if your DBMS does not support parameterized queries. In that case, find another DBMS.

CaptainObvious:
I concat strings for SQL execution all the time.

I'm sorry to hear that. I did that, too, at one time, but I was clueless and ignorant.

CaptainObvious:
System.String.Format("SELECT * FROM Foo WHERE Bar={0}", SQLExcape(szUserInput));

The sad, if not tragic part is that you could have used almost the same syntax, just with placeholders like "@0" and used ADO.NET's built-in SQL parameter support instead.

Just because SQL-escaping "solutions" are better than using unescaped user input does not mean they are good.

First, how do you know you have the syntax covered? Some databases accept only single quotes to delimit strings, others accept double quotes, some allow even other delimiters, and not all of them use the same escape syntax. Are you sure that there's no way to bypass your escaping code?

Second, using dynamic SQL for frequently-used, static queries is inefficient because the query has to be parsed and planned over and over again, for nothing. Although some RDBMSes can cache query plans for pseudo-dynamic queries, because they expect to be used by morons, they still have to do a redundant parse and the caching overhead will never be as easily tunable as prepared statements (SQLCommand.Prepare() in ADO.NET).

Third, just like the redundant parsing overhead in the DBMS, there is redundant formatting overhead in the application, e.g. turning a binary number into a decimal string and back. With parameterized queries, OTOH, the database driver can select the most efficient way to transmit the query parameters. And it will not even need to transfer the query's structure if the statement has been prepared on the server.

Fourth, pseudo-dynamic SQL makes it very easy to fuck up the syntax with locale bugs. By not using CultureInfo.InvariantCulture, your own example very likely exhibits this problem! Unless the server's locale expectations happen to be 100% compatible with the locale your application is using, you will get screwed. Floating-point numbers and dates are the number one examples, e.g. if your app is using a German locale (or Argentinian, Austrian, Belgic, Brazilian, French, Russian, Swiss, Turkish and others), it will format floating-point numbers with a decimal comma, which will typically totally confuse the database system.

Fifth, in ADO.NET, using pseudo-dynamic SQL means you have to create a new command object for every query with the same structure. Instead of creating and preparing the command once and executing it as often as necessary with different parameters. This is, again, inefficient.

Sixth, if you are using dynamically typed parameters, a type mismatch will generate a clearly understood error when using paramterized queries (e.g. InvalidCastException in ADO.NET), while using pseudo-dynamic SQL will often just generate an unspecific "syntax error".

All that said, I'd like to hear just one reason for not using parameterized queries instead. Because your posting doesn't mention any.

2007-03-24 Reply Admin

Ted:
So, has anybody tried it...? <G>
p.s. LOL! My captcha for this comment is "dreadlocks" -- never had a "good" word, before...

You still don't, dude, you still don't.

2007-03-24 Reply Admin

Unelegant! How about:

bool breakOut = false;

while (!breakOut) {
try {
   this.RunTest();
}

catch(Exception e) {
   if(e.Message.IndexOf("SA Application") != -1) {
      continue;

   }  else {
      breakOut = true;
      throw e;
   }
}

}

PSWorx · 2007-03-24 Reply Admin

AdT:
CaptainObvious:
Jeff S:
to protect against injection, all you have to do is use parameterized commands. You should *never* concatenate anything together with a SQL statement and execute it, unless the language/framework/database you are using doesn't support parameters. Of course, in that case, you probably have bigger issues to worry about.

Never is such a horrible word to use.

But in this case, it's absolutely appropriate. NEVER EVER use dynamic SQL techniques to generate static SQL. Not even if your DBMS does not support parameterized queries. In that case, find another DBMS.

CaptainObvious:
I concat strings for SQL execution all the time.

I'm sorry to hear that. I did that, too, at one time, but I was clueless and ignorant.

CaptainObvious:
System.String.Format("SELECT * FROM Foo WHERE Bar={0}", SQLExcape(szUserInput));

The sad, if not tragic part is that you could have used almost the same syntax, just with placeholders like "@0" and used ADO.NET's built-in SQL parameter support instead.

Just because SQL-escaping "solutions" are better than using unescaped user input does not mean they are good.

First, how do you know you have the syntax covered? Some databases accept only single quotes to delimit strings, others accept double quotes, some allow even other delimiters, and not all of them use the same escape syntax. Are you sure that there's no way to bypass your escaping code?

Second, using dynamic SQL for frequently-used, static queries is inefficient because the query has to be parsed and planned over and over again, for nothing. Although some RDBMSes can cache query plans for pseudo-dynamic queries, because they expect to be used by morons, they still have to do a redundant parse and the caching overhead will never be as easily tunable as prepared statements (SQLCommand.Prepare() in ADO.NET).

Third, just like the redundant parsing overhead in the DBMS, there is redundant formatting overhead in the application, e.g. turning a binary number into a decimal string and back. With parameterized queries, OTOH, the database driver can select the most efficient way to transmit the query parameters. And it will not even need to transfer the query's structure if the statement has been prepared on the server.

Fourth, pseudo-dynamic SQL makes it very easy to fuck up the syntax with locale bugs. By not using CultureInfo.InvariantCulture, your own example very likely exhibits this problem! Unless the server's locale expectations happen to be 100% compatible with the locale your application is using, you will get screwed. Floating-point numbers and dates are the number one examples, e.g. if your app is using a German locale (or Argentinian, Austrian, Belgic, Brazilian, French, Russian, Swiss, Turkish and others), it will format floating-point numbers with a decimal comma, which will typically totally confuse the database system.

Fifth, in ADO.NET, using pseudo-dynamic SQL means you have to create a new command object for every query with the same structure. Instead of creating and preparing the command once and executing it as often as necessary with different parameters. This is, again, inefficient.

Sixth, if you are using dynamically typed parameters, a type mismatch will generate a clearly understood error when using paramterized queries (e.g. InvalidCastException in ADO.NET), while using pseudo-dynamic SQL will often just generate an unspecific "syntax error".

All that said, I'd like to hear just one reason for not using parameterized queries instead. Because your posting doesn't mention any.

Simply beautiful. Nothing to add. Thanks man.

Jeff S · 2007-03-24 Reply Admin

Great post, ADT! Please register and stick around and contribute more often, we need more like you!

2007-03-24 Reply Admin

I know a guy with the last name "null" who once told me that he has from time to time had difficulty where he had to be added to a database. I like to give him a hard time and tell him our HR database doesn't allow Nulls.

2007-03-24 Reply Admin

Let's get real, so you think that precise code is executed on Dice's server.

We hope not. Though of course it might be.

It's just a fscking advertisement, more of joke than anything else.

2007-03-24 Reply Admin

AdT:
All that said, I'd like to hear just one reason for not using parameterized queries instead. Because your posting doesn't mention any.

Statements with variable tables have to be dynamic, at least the table names must be inserted as strings. Reusing the query plan only works if the tables stay the same, anyway. And if you use the query multiple times.

CREATE TABLE/CREATE INDEX/ALTER TABLE/ALTER SEQUENCE may need values as parameters, e.g. partition criteria or default values. Are they parametrizable in every database? Do they count as 'queries'?

Variable length lists like "WHERE a IN (?,?,?,...,?)" where you have the choice between typing in or dynamically creating 'all' variants of the template. Or loading the values into a temporary table and JOINing.

Other complicated logic may need to generate WHERE clauses totally dynamic depending on user input or config files (e.g. formulas or complicated criteria). It may be less error prone to directly insert the values and not construct a template string and parameter list independently.

Some database protocols don't support parameters and the client side drivers have to fill in the parameters by string concatenation anyway. Depending on the circumstances it may be faster to do the concatenation yourself, e.g. if you call the statement multiple times with the same parameters.

Good client drivers have an encode() or quote() method that solves all locale and encoding problems because it does the Right Thing in any case.

Sending PREPARE, receiving the statement handle, sending parameters and receiving the result are 2 network round trips. Just sending a complete statement and receiving the result is 1. 50% speed up.

2007-03-25 Reply Admin

When you say spectrum analyzer I hope you mean ZX spectrum, because with anything more serious in your hands, you'd be dangerous.

Formal constructs like "while" were devised for this exact purpose.

bool bContinue = true; while(bContinue) { try {this.runtest()} catch(Exception e) { if(!e.Message.contains("SA Application")) {bContinue = false; throw e;} } }

With the exception of VB6 error handling ... THERE IS NEVER ANY REASON YOU SHOULD USE A GOTO STATEMENT ... ESPECIALLY IN C#!!! (sorry for raising my voice - but really)

In fact here's an article from 1968 (reprinted '95) that warned about them ... nearly 40 yrs ago!!!

http://www.acm.org/classics/oct95/

captha: 10 PRINT "WTF?" 20 GOTO 10

2007-03-25 Reply Admin

jiggles:
bool bContinue = true; while(bContinue) { try {this.runtest()} catch(Exception e) { if(!e.Message.contains("SA Application")) {bContinue = false; throw e;} } }

This is a good example that GOTO control flow can be more comprehensible then ideologically placed loops. Being readable is all that counts in programming.

You noticed that the loop is infinite as long as no harmful Exception (a real device error or something like that) is raised, and even then you assigned bContinue for no reason because you leave the loop by rethrowing the exception? On purpose?

2007-03-25 Reply Admin

Paul:
Doesn't SQL normally use double-quotes for object names, and single-quotes for string literals? So the ad doesn't make sense anyways?

hey, this has been written by advertisers at the sundeck, not by programmers at the machineroom. ;)

Jeff S · 2007-03-25 Reply Admin

strcmp:
AdT:
All that said, I'd like to hear just one reason for not using parameterized queries instead. Because your posting doesn't mention any.

Statements with variable tables have to be dynamic, at least the table names must be inserted as strings. Reusing the query plan only works if the tables stay the same, anyway. And if you use the query multiple times.

Still no reason to not use parameters. You can very easily build a complete dynamic SQL statement with parameter placeholders and assign values and datatypes for all of the advantages that I spoke about in my two posts here. The table name itself cannot be a parameter, but in this case, it doesn't matter - the table name is NOT data. It does not have a data type and doesn't have different formats according to culture. It is a table name, part of the SQL statement. If your design requires variable table names then, again, you have bigger problems rather than worrying about parameters, I suppose, but that does NOT stop you from using parameters for your DATA. The whole issue with parameters versus concatenation is the distinction between DATA and CODE, which people seem to confuse, which using and understanding parameters really helps to differentiate.

CREATE TABLE/CREATE INDEX/ALTER TABLE/ALTER SEQUENCE may need values as parameters, e.g. partition criteria or default values. Are they parametrizable in every database? Do they count as 'queries'?

No. See the previous post. And please try to recognize the different between administrative sql statements that are executed versus those that your apps use during runtime to function, and also try to recognize the different between the DATA and the CODE portions of a sql command.

Variable length lists like "WHERE a IN (?,?,?,...,?)" where you have the choice between typing in or dynamically creating 'all' variants of the template. Or loading the values into a temporary table and JOINing.

Still no reason. as mentioned, you can easily use (@p1,@p2,@p3) and set parameters, and now you get all the benefits of no need to format or convert or delimit or escape and DATA is passed as native VALUES instead of everything being converted and formatted as strings.

Other complicated logic may need to generate WHERE clauses totally dynamic depending on user input or config files (e.g. formulas or complicated criteria). It may be less error prone to directly insert the values and not construct a template string and parameter list independently.

ONce again, no reason. I demonstrate this exact thing in my blog post (the URL is in my first post, no need to spam and print it again).

To briefly demonstrate how simple it is:

if (country != '') { sqlcommand.CommandText += ' and Country=@Country '; sqlcommand.Parameters.Add("@Country", sqldbtype.Varchar) = country; }

could it be any easier? And guess what, let's beat a dead horse and again remind you that not only are we 100% safe for injection, not only is the code still short and clear and simple, but now we can reference @Country as many times as we want in our sql statement, the data is passed BY VALUE and doesn't need to be escaped and delimited and so on.

Some database protocols don't support parameters and the client side drivers have to fill in the parameters by string concatenation anyway.

That's fine, if your client or server don't support parameters, then that's a whole different thing of course. That's me saying "always use exceptions to handle errors" and you replying "but some languages don't have exceptions!" . I think it should be obvious that when I make that statement, it is implied that you should always use them when the language supports them.

Depending on the circumstances it may be faster to do the concatenation yourself, e.g. if you call the statement multiple times with the same parameters.

No. if you want to call the statement over and over, even with parameters, you set 'em once and call the statement over and over. Done. More often, you need to just alter a single parameter and call that statement over and over, and this is much easier, faster and more efficient with parameters -- you create the command once, and set the parameter for each call.

Good client drivers have an encode() or quote() method that solves all locale and encoding problems because it does the Right Thing in any case.

Same deal as before, continuing to beat that dead horse some more, it doesn't matter what kind of functions you use to encode or quote or decode things at the client -- If you DON'T use parameters, you are NOT passing VALUES -- you are passing strings to the database -- always! Why would you do this? What advantage do you gain by this? None! Absolutely nothing, just more code, more converting, more formatting, more overhead, more places where things can do wrong. Why would you introduce this complexity and overhead to your code when you simply do not need to?

Sending PREPARE, receiving the statement handle, sending parameters and receiving the result are 2 network round trips. Just sending a complete statement and receiving the result is 1. 50% speed up.

There is no need to prepare a sql command if you want to use parameters. You can do this if you want, but you don't need to do -- that is NOT a requirement of using parameters. And, hopefully, you understand that the amount of data send via the sql statement is minuscule compared to the amount of data returned! Simply adding 1 more tiny round trip to prepare something that eventually returns 4,000 rows of data does not decrease the efficiency by 50% !

...

Good try, I suppose ... you sound knowledgeable on the topic, so I have to ask: Do YOU use parameters? I sure hope so, since typically they are not used by people who tend to be a little more ignorant. Hopefully, you are just grasping at straws and playing a little devil's advocate here.

So, the fact remains: ALWAYS use parameters.* Even better, use stored procedures. But even if you don't use stored procs, ALWAYS use parameters. It's simpler, easier, often shorter, clearer, and so on. It's just plain stupid not to use them.

And let's be clear: No one is saying that you should never execute any sql statement from the client w/o using parameters, of course sometimes you don't need them and there's always odd cases (DDL commands, DBA maintenance commands, some overly flexible reporting apps) here and there; the point is that as a RULE, as a general PRACTICE, when your clients interact with your database, they should always pass DATA using parameters and not convert and escape everything into strings via concatenation. It's that simple.

when your db and client frameworks support them, of course.

real_aardvark · 2007-03-25 Reply Admin

You know, I've never really understood why modern computer languages don't have syntactic support for "Halt and Catch Fire." Several very influential architectures incorporated a variant on that instruction. (!<snopes/>.)

This probably explains why we have the Wintel world of today, with its mealy-mouthed "goto"-based assembler, rather than a MotorMac world, with a far more appealing "Halt and Catch Fire"-based assembler.

themagni:
Aaron:
Yes - write a subroutine. They came up with this concept in the 1960s (or was it the 1950s?) so people wouldn't have to continue writing GOTOs.
GOTO is a valid instruction on most (all?) architectures.
The End.

No, "The End" would be Halt and Catch Fire. Not GOTO. Oh no, dearie me. GOTO is not The End. It's more like a cliff-hanger, as in "Tune In Next Week..."

Although I do see your point. A computer language without a goto is like a fish without a bicycle ... er ... well, lisp was never very successful, was it? And now we know why. What an absurd oversight. How could it possibly call itself (well, I suppose that's infinite recursion, but then lisp is good at that) a "functional" language?

Julian:
I've never been able to avoid a code elegance question. So how about this: <excellent code snipped/> Which employs tail-recursion to stop filling up the stack.
Of course, there are good reasons there is a goto in the language (try breaking out of a nested loop sometime). But to be honest the original formulation was pretty clear. It didn't have any serious problems. Hence, I claim it's good code (even if I've just written two variants on it...) :)

Even cultured and obviously smart people seem bewitched by this hideous Wintel conspiracy.

boolean WTF = true;
for (;;) { // formerly "while (WTF) {"
     do {
         printf ("Help! I'm stuck in a nested loop, and the warranty on my goto has run out!\n");
     } while (1); // formerly "} while (WTF);"
}

It really is no wonder that compilers complain about this sort of thing.

PSWorx · 2007-03-25 Reply Admin

real_aardvark:
You know, I've never really understood why modern computer languages don't have syntactic support for "Halt and Catch Fire." Several very influential architectures incorporated a variant on that instruction. (!<snopes/>.)
This probably explains why we have the Wintel world of today, with its mealy-mouthed "goto"-based assembler, rather than a MotorMac world, with a far more appealing "Halt and Catch Fire"-based assembler.

themagni:
Aaron:
Yes - write a subroutine. They came up with this concept in the 1960s (or was it the 1950s?) so people wouldn't have to continue writing GOTOs.
GOTO is a valid instruction on most (all?) architectures.
The End.

No, "The End" would be Halt and Catch Fire. Not GOTO. Oh no, dearie me. GOTO is not The End. It's more like a cliff-hanger, as in "Tune In Next Week..."

Although I do see your point. A computer language without a goto is like a fish without a bicycle ... er ... well, lisp was never very successful, was it? And now we know why. What an absurd oversight. How could it possibly call itself (well, I suppose that's infinite recursion, but then lisp is good at that) a "functional" language?

Julian:
I've never been able to avoid a code elegance question. So how about this: <excellent code snipped/> Which employs tail-recursion to stop filling up the stack.
Of course, there are good reasons there is a goto in the language (try breaking out of a nested loop sometime). But to be honest the original formulation was pretty clear. It didn't have any serious problems. Hence, I claim it's good code (even if I've just written two variants on it...) :)

Even cultured and obviously smart people seem bewitched by this hideous Wintel conspiracy.
boolean WTF = true;
for (;;) { // formerly "while (WTF) {"
     do {
         printf ("Help! I'm stuck in a nested loop, and the warranty on my goto has run out!\n");
     } while (1); // formerly "} while (WTF);"
}
It really is no wonder that compilers complain about this sort of thing.

No Quack? o.O

2007-03-25 Reply Admin

strcmp, you're right about readable, I missed the BBCode thang :(

strcmp:
even then you assigned bContinue for no reason because you leave the loop by rethrowing the exception? On purpose?

Yes, for completeness sake, paranoia and knowing that I have to change it tomorrow to include some other madness that means not throwing the exception further out. I shouldn't have shown it throwing the exception ... perhaps "//todo" might have been a more clear example - to be honest I hadn't had any coffee by the time I wrote it ;)

Some would call it a 'WTF?' ... fair point

2007-03-25 Reply Admin

[Injection is bad]

Did anybody consider that the user which executes the said statement against the DB only has the right to read? Or what would be wrong with utilizing simply the daabase-security-measures to protect its content?

2007-03-25 Reply Admin

Maybe the are looking for the one that actually does the injection :) that person realy knows something about SQL injection :) It's just like hiring a Hacker. If you can’t beet then Hire them!

2007-03-25 Reply Admin

doing queries like that is actually ok if they properly validate all input, so even if that was a real query there's absolutely nothing wrong with it

so get a clue please.

2007-03-25 Reply Admin

panzi:
If this is actually how you do job searches on their site, they may want to add a disclaimer; Please do not enter the following in either field: ";DELETE FROM JOBS;SELECT * FROM JOBS WHERE "1" = "1

Is this a joke or does it really work??

Highly unlikely. Most likely the Dice site is using PHP and running submitted text through mysql_real_escape($job_type), or the postgreSQL equivalent before making any use of it, so unless the job type description includes attempted SQL injection code, they would simply return an empty list.

2007-03-25 Reply Admin

strcmp:
Statements with variable tables have to be dynamic, at least the table names must be inserted as strings.

For one, please tell me you do not use tables like "OrdersJan07"... or that the table names are based on user input (unless you're writing a DB administration tool).

Anyway, tables are database objects and their names are not values. Parameter placeholders are used to insert values, so what you want to do is create dynamic SQL. If you had read my post carefully, you would have noticed that I did not say "never use dynamic SQL", but never to use string formatting to generate static SQL - SQL with a fixed structure and different values.

That's not to say that dynamic SQL isn't over(ab)used. It is. I just want to say there are legitimate reasons to use dynamic SQL even if in at least 80% of the cases, it's neither necessary nor very wise. However, even if you use dynamic SQL, there are excellent reasons to use parameters for all values (as opposed to SQL structure, including table and field names).

This should answer most of the rest of your post.

strcmp:
Other complicated logic may need to generate WHERE clauses totally dynamic depending on user input or config files (e.g. formulas or complicated criteria).

That sounds like an invitation for abuse.

strcmp:
It may be less error prone to directly insert the values and not construct a template string and parameter list independently.

And then it may not...

strcmp:
Some database protocols don't support parameters and the client side drivers have to fill in the parameters by string concatenation anyway. Depending on the circumstances it may be faster to do the concatenation yourself, e.g. if you call the statement multiple times with the same parameters.

It may also be faster to manipulate NTFS using direct sector writes instead of using the Windows Filesystem API.

strcmp:
Good client drivers have an encode() or quote() method that solves all locale and encoding problems because it does the Right Thing in any case.

If the client driver is good, you don't need these in the first place because it will provide support for the more efficient and secure parameterized commands.

strcmp:
Sending PREPARE, receiving the statement handle, sending parameters and receiving the result are 2 network round trips. Just sending a complete statement and receiving the result is 1. 50% speed up.

First, please tell me where I suggested that you should prepare every statement. I said that you can prepare frequently used static statements when using parameters, but you cannot do that when injecting values into template strings. Yes, I said "frequently used" and "static", literally. I wonder how you managed not to notice.

Second, the 50% figure assumes that the statement is prepared but then only used once, which would be unusual for static statements in most applications. In the case of dynamic SQL, you would execute a parameterized command without preparing it first. Is that too obvious?

2007-03-25 Reply Admin

Leo:
Did anybody consider that the user which executes the said statement against the DB only has the right to read?

Then it might still be a confidentiality disaster.

Let's pipe "SELECT email FROM customers" to the spambot.

Leo:
Or what would be wrong with utilizing simply the daabase-security-measures to protect its content?"

Pardon? In this case you would need a database user for every application user and per-row-ACLs, which few databases support (and which are horrible for performance, anyway).

2007-03-25 Reply Admin

Jeff S:
CaptainObvious:
Jeff S:
to protect against injection, all you have to do is use parameterized commands. You should *never* concatenate anything together with a SQL statement and execute it, unless the language/framework/database you are using doesn't support parameters. Of course, in that case, you probably have bigger issues to worry about.

Never is such a horrible word to use. I concat strings for SQL execution all the time, well, I use System.String.Format, actually, but same effect:

System.String.Format("SELECT * FROM Foo WHERE Bar={0}", SQLExcape(szUserInput));

Now, concatenating raw user input to SQL without escaping it, thats just stupid... Although I do have that one application I use for executing arbitrary SQL, that is by its nature an acceptable use of unescaped SQL, but there is no concatenation involved then, so it doesn't qualify.

I stand by it. Never. Ever.

see: http://weblogs.sqlteam.com/jeffs/archive/2006/07/21/10728.aspx

When you don't use parameters, you are not passing VALUES to the SQL server -- you never pass anything other than STRINGS. Which means that all data to be passed must first be formatted and converted to strings, and then sql must convert the data from a string back to the correct datatype. This means that suddenly date and numeric and boolean (bit) formats at the client versus the server become very important, since EVERY VALUE that gets passed to the database must be formatted as string. i.e., your data goes from the native types at the client to a strings which must be converted back to a native type by the server. Does this make sense? Why would you do this if you didn't have to?

Using parameters is quick, short, easy, safe, and correct.
It is actually NOT lazy to "avoid" parameters, it is actually easier to use parameters and experience much less of a headache, for all the reasons I demonstrate in my article. Using String.Format() is easier than direct concatenation, but things STILL must always be converted and delimited and escaped as strings, and you loose the benefit of quickly looking at your sql code to see @UserID and @AccountNumber sprinkled throughout instead of {0} and {1}.

Of course, using stored procs is even better. but we won't go there .... after all, that is debatable, I suppose, and it certainly has been quite a bit over the years. But whether or not to use parameters simply isn't debatable.

Be smart. Do it right. Do it quick. Do it easily. Write clearer code ... and use parameters. There are very few "always" and "nevers" when it comes to programming, but this is one of them.

Well Jeff,

Your argument would be much more convincing if the sample code you used were error=free and tested. I just took a side-jaunt off to follow your link, and had a major WTF!?!? moment when I saw the "right way to do it" sample code blithely data intended for an ID column to a column named @TYPE.

I you have time to write te lengthy screed, you could at least find some time to test your code against some actual test data. It's one thing to have typos when writing into these screwy undersized textareas on thedailywtf.com. It's just plain lazy to not check the integrity of your smples when holding forth as the expert on your own blog.

/Captcha -- gotcha indeed, or you set yourself up for it.

Jeff S · 2007-03-25 Reply Admin

Joseph Newton:
Well Jeff,
Your argument would be much more convincing if the sample code you used were error=free and tested. I just took a side-jaunt off to follow your link, and had a major WTF!?!? moment when I saw the "right way to do it" sample code blithely data intended for an ID column to a column named @TYPE.

I you have time to write te lengthy screed, you could at least find some time to test your code against some actual test data. It's one thing to have typos when writing into these screwy undersized textareas on thedailywtf.com. It's just plain lazy to not check the integrity of your smples when holding forth as the expert on your own blog.

/Captcha -- gotcha indeed, or you set yourself up for it.

Wow. you got me. A cut and paste typo in my SAMPLE code which is simply an EXAMPLE and not designed or intended to run in any specific context (where's the using statements? the variable declarations? the call to the connection open method? the error handling? the execute of the sqlcommand? etc...) completely negates my general point about a programming concept. Damn, I am an idiot. You got me.

Well, I guess I should say thanks for letting me know, and also, the typo has been fixed. Also, you will be happy to know that I've fired my editor and will take a self-imposed 5% pay cut from my blog income as a penalty for this horrendous oversight. I apologize for any inconvenience that this may have caused. It's a shame that something like this makes my entire point about parameters moot ... oh well ...

Now Hiring SQL Injectors

Leave a comment on “Now Hiring SQL Injectors”