Comment On Erring On The Side of Danger

The user id & password is handy.

Why bother development? Just let your users log in to the database and fix the data directly.

looks like instead of doing any error handling, they decided to not do any at all so you get the wonderful asp.net stack trace

SQL Security has always scared me, although I guess it's needed sometimes.. If you're only coding against Windows, and running SQL Server, why not use Integrated Security?

(Regardless)

Let hope cmsadmin isn't a true admin account, if people are sharing it. I cringe if they are actually a DBOwner.. Ugh!

And sharing a connection / login is horrible for security! How can you tell who is accessing what data?

I think his mother was a hamster, and his father smelt of elderberries...

 

I guess people who don't speak Dutch won't get it, but that is an odd
choice for a password. Does the admin have a thing for speed cameras?

Kippesoep:

I guess people who don't speak Dutch won't get it, but that is an odd choice for a password. Does the admin have a thing for speed cameras?

Heh -- actually all conn string info was changed by yours truly. But this does confirm that no matter what random typing I do, I can write offensive words ... in some language ... somewhere. Grschepen!

Yike!
Clear text database name, user ID AND password in the error message... priceless.

Why not just use 'sa'?  Then you don't have to worry about transmitting a password to the user.

Sean:

Why not just use 'sa'?  Then you don't have to worry about transmitting a password to the user.

my thoughts exactly... than they won't be able to retrieve the password either...

John Smallberries:

The user id & password is handy.

Why bother development? Just let your users log in to the database and fix the data directly.

Well, given the target data source,

data source=DTESQL04.INITECH-GLOBAL.COM

Why should the users stop at molesting the data?

I think that the users should probably go ahead and log in as admins, to fix the data model, as well.

Alex Papadimoulis:

Heh -- actually all conn string info was changed by yours truly.

Was the original password something like... "cmsadmin" ?

I don't think the connnection thing is necessarily a WTF.

For example, activeConnections could be a property that returns a separate instance depending on the user log in name from integrated security for example. Or it could be a WinForms application where there is only one user at a time.

The keyword this inside a static method puzzles me though. Is that valid C#? Or maybe an Alex-typo?

I guess people who don't speak Dutch won't get it, but that is an odd choice for a password. Does the admin have a thing for speed cameras?

From flTSP4#1 to Flitspaal... It requires some imagination on your part, though! By the way, is it me, or does this board have a rather high percentage of Dutch speaking people? Go low countries, go!

Alex Papadimoulis:

As it turns out, the offending code really wasn't bad. It just neglected to account for a situation where more than one person would be using the application at the time ....

Actually it looks like a lovely way to replicate DB2 locking on a SQL server backend. 

If bAlreadyInUse THEN Error

I like it!

Anonymous:

I don't think the connnection thing is necessarily a WTF.

For example, activeConnections could be a property that returns a separate instance depending on the user log in name from integrated security for example. Or it could be a WinForms application where there is only one user at a time.

The keyword this inside a static method puzzles me though. Is that valid C#? Or maybe an Alex-typo?

Whoops ... my bad. I added "this." for clarification during the normal anonymozing of course forgetting that you can't do that in a static method. And activeConnections is a Hashtable.

Leaving aside the reason you would get this error message - "what do you mean more than one person wants to access the website at once?" I'm hoping that the programmer just forgot to set

<customErrors mode="On" />

in the Web.config file.

 

Note: lets see if the forum handles HTML properly from SharpReader....[^o)]

Not sure I am fully understanding what's happening in here - the only thing I can figure is that the value of connectionString must be affected by the CreateConnection function.  He's checking to make sure that connectionString isn't in the dictionary before calling CreateConnection - and then after calling CreateConnection, the connectionString magically IS in the dictionary?

DelawareBoy:

SQL Security has always scared me, although I guess it's needed sometimes.. If you're only coding against Windows, and running SQL Server, why not use Integrated Security?

I agree completely - Integrated security is better in most cases.  However, as you say, it's sometimes needed, for example, if the windows users don't have network rights from the client machine to the server machine (usually this happens if the machines are on different domains), or if people from the same windows group need different rights in the database, or if you're not using SQL Server (there's no provider element to the tag, which suggests they're using the SQL Server connection object).

In this case, judging by the machine name, it's on a different domain, so they need to use SQL security.

But yeah, I don't know what insight the user is expected to glean from this error.

Anonymous:

I guess people who don't speak Dutch
won't get it, but that is an odd choice for a password. Does the admin
have a thing for speed cameras?

From flTSP4#1 to Flitspaal... It requires some imagination on your
part, though! By the way, is it me, or does this board have a rather
high percentage of Dutch speaking people? Go low countries, go!

Hoera voor de lage landen!



(I.e.: Hoorah for the low countries)



Booh to the programmer who wrote this WTF.

Anonymous:

For example, activeConnections could be a property that returns a separate instance depending on the user log in name from integrated security for example. Or it could be a WinForms application where there is only one user at a time.

It's pretty clear from the code and the description of the problem that neither of these are true.

Look closely: it's a synchronization issue.  A pretty typical race condition.

Alex Papadimoulis:

Heh -- actually all conn string info
was changed by yours truly. But this does confirm that no matter what
random typing I do, I can write offensive words ... in some language
... somewhere. Grschepen!

It wasn't actually offensive. As was posted by somebody else,
"flitspaal" is simply our word for speed camera. Could've been a real
password, as many people take a word they know and apply some leetspeek
to it. Easier to remember that way.



That said, your new random word makes you sound like you're mad at
ships ("schepen" being the Dutch word for "ships"). I guess that proves
your theorem.

skicow:

Leaving aside the reason you would get this error
message - "what do you mean more than one person wants to access the
website at once?" I'm hoping that the programmer just forgot to set

<customErrors mode="On" />

in the Web.config file.

I have to admit the error already showed up during the internal testing
stage, and in production the SQL server isn't accessible to IP's other
than local.



But ofcourse my thoughts and condoleances are with the ones setting
this app up in a shared hosting environment without setting the
customErrors on. (and the ones without the source code license)



The irony here is that the key was only used to store a key/value pair, and the hashtable would only contain 1 value.



So the value could have been any other constant (other than just NOT
putting it in a HashTable but a regular variable), but they chose to
use the application's connection string, just not to declare another
const.

John:

Not sure I am fully understanding what's happening
in here - the only thing I can figure is that the value of
connectionString must be affected by the CreateConnection
function.  He's checking to make sure that connectionString isn't
in the dictionary before calling CreateConnection - and then after
calling CreateConnection, the connectionString magically IS in the
dictionary?

The programmer is not properly handling global data.  It assumes
only one execution thread, and thus there is a race condition between
the ContainsKey check and the Add check that results in the
error.  To fix just this error (but not the WTF at large), he
should be putting a lock around activeConnections.

Anonymous:

Not sure I am fully understanding what's happening in here - the only thing I can figure is that the value of connectionString must be affected by the CreateConnection function.  He's checking to make sure that connectionString isn't in the dictionary before calling CreateConnection - and then after calling CreateConnection, the connectionString magically IS in the dictionary?

Alex was referrering to the lack of concurrency handling (e.g. locking the hashtable during this).  Although it pains me to search the hashtable twice to do one insert, this would have prevented this situation:

protected static void EnsureConnection(ConnectionProvider provider, string connectionString)

{

  if (activeConnections.ContainsKey(connectionString))
      return;

  lock(activeConnections)

  {
      if (!activeConnections.ContainsKey(connectionString))
      {

          DataConnection conn = CreateConnection(provider.ConnectionType); 

          activeConnections.Add(connectionString, conn);
      }

  }

}

Anonymous:

Not sure I am fully understanding what's happening in here - the only thing I can figure is that the value of connectionString must be affected by the CreateConnection function.  He's checking to make sure that connectionString isn't in the dictionary before calling CreateConnection - and then after calling CreateConnection, the connectionString magically IS in the dictionary?

It's a common multithreading issue.  Two threads come along to call this method at approximately the same time (on a computer scale.)  They both check to see than there's no entry in the hashtable for the key.  They both see an answer of no and try to insert something into it.  Generally, one will be slightly ahead of the other. The thread that inserts first succeeds and the thread that inserts second fails.

skicow:

Leaving aside the reason you would get this error message - "what do you mean more than one person wants to access the website at once?" I'm hoping that the programmer just forgot to set

<customErrors mode="On" />

in the Web.config file.

 

Note: lets see if the forum handles HTML properly from SharpReader....[^o)]

unfortunately that customErrors tag doesn't always behave correctly...

Anonymous:

Anonymous:

I guess people who don't speak Dutch
won't get it, but that is an odd choice for a password. Does the admin
have a thing for speed cameras?

From flTSP4#1 to Flitspaal... It requires some imagination on your
part, though! By the way, is it me, or does this board have a rather
high percentage of Dutch speaking people? Go low countries, go!

Hoera voor de lage landen!



(I.e.: Hoorah for the low countries)



Booh to the programmer who wrote this WTF.

And totally off the topics - some thing that the Dutch guys
will also get – down in S Africa we call the
people who put up a ‘Flitspaal’ (cool I like it) i.e. traffic officers ‘Bose
Geeste’. 

 As for the actual topic – scary code once again.  The error can be forgiven as even I made a similar
mistake 25 years ago when I started programming – but the ID & Password –
This programmer should be send back to college to go and do System Security
101.

And this is why blindly following idioms might not be the best of ideas all the time. Here, catch-don't-check would have been better than check-don't-catch, despite exceptions being expensive to throw.

I'm boggling over the way this is done, though. I'm not a fan of remote populating collections or arrays in methods of return type void, but then that may be personal preference more than anything else.

Anonymous:

Hoera voor de lage landen!

(I.e.: Hoorah for the low countries)

Booh to the programmer who wrote this WTF.

There are indeed quite a lot of Dutch speaking people in here (like me! - From Belgium, nonetheless). The WTF babe is a Dutch girl, but I haven't seen her in a while. Is she still here?

This board is very Dutch, apparently.



*raises hand*



And now, an extended Dutch translation of common BLOCKED SCRIPT

geschrift.haalElementOpAanDeHandVanIdentiteit();
geschrift.haalElementenOpAanDeHandVanElementNaam();

geschrift.haalElementenOpAanDeHandVanElementNaam('form')[0].versturen()

var
eersteHoogte = geschrift.haalElementenOpAanDeHandVanElementNaam('div')[0].kindKnopen[0].verplaatsingafstandHoogte;

dialoogVenster('hallo wereld');

geschrift.haalElementOpAanDeHandVanIdentiteit('elementje').binnensteHTML =
'iets';

geschrift.schrijf('pompipom');

This forum blocks out the word 'java-script' even though there is no
situtation in which the plain word could do any damage whatsoever.



What?

It seems nobody gets it: the user/pass-message is clearly coming from .NET, probably from the DB-driver (.NET JDBC equivalent). Yet another security short sight from Mickey-culture.

Anonymous:

It seems nobody gets it: the user/pass-message is clearly coming from .NET, probably from the DB-driver (.NET JDBC equivalent). Yet another security short sight from Mickey-culture.

I hate to burst your 'randomly attacking Microsoft' bubble, but the user/pass is in the connectionString passed into the method. The HashMap is complaining about the duplicate key, not the value.

Alex Papadimoulis:

Kippesoep:

I guess
people who don't speak Dutch won't get it, but that is an odd choice
for a password. Does the admin have a thing for speed cameras?

Heh -- actually all conn string info was changed by yours truly. But
this does confirm that no matter what random typing I do, I can write
offensive words ... in some language ... somewhere. Grschepen!

HAHAHA!  You've made my day.  That's even better than the WTF today :)



    -dZ.

phelyan:

Anonymous:

It seems nobody gets it: the user/pass-message is clearly coming from .NET, probably from the DB-driver (.NET JDBC equivalent). Yet another security short sight from Mickey-culture.

I hate to burst your 'randomly attacking Microsoft' bubble, but the user/pass is in the connectionString passed into the method. The HashMap is complaining about the duplicate key, not the value.

I think he was complaining about the username and password appearing in the error message. Of course, he's still talking rubbish - AFAIK, most database access engines, not just ADO.NET, will do that if you ask them to.  Besides, it's the programmers' mistake to include the whole string in the message, not Microsoft's.

It looks like the programmer decided to use the hash table to make sure he had no duplicate connection strings (by putting the value of the connection string in as the key, it'll throw an error if you get two the same).  This is a WTF in itself, since if those strings ever change then I don't think it'll allow you to, er, rename the key.  You'd have to remove it and add it again, I suppose.

Anyway, if you want to do that kind of thing, then you should have a catch handler which strips out all the sensitive information.

Verbose

phelyan:

Anonymous:

It seems nobody gets it: the user/pass-message is clearly coming from .NET, probably from the DB-driver (.NET JDBC equivalent). Yet another security short sight from Mickey-culture.

I hate to burst your 'randomly attacking Microsoft' bubble, but the user/pass is in the connectionString passed into the method. The HashMap is complaining about the duplicate key, not the value.

I think he was complaining about the username and password appearing in the error message. Of course, he's still talking rubbish - AFAIK, most database access engines, not just ADO.NET, will do that if you ask them to.  Besides, it's the programmers' mistake to include the whole string in the message, not Microsoft's.

It looks like the programmer decided to use the hash table to make sure he had no duplicate connection strings (by putting the value of the connection string in as the key, it'll throw an error if you get two the same).  This is a WTF in itself, since if those strings ever change then I don't think it'll allow you to, er, rename the key.  You'd have to remove it and add it again, I suppose.

Anyway, if you want to do that kind of thing, then you should have a catch handler which strips out all the sensitive information.

Verbose errors

Dammit, tried typing after clicking post!  How's that for a WTF?  [:P]

Anyway, verbose errors are great for debugging, but a user will just be confused by them.

A user can't diagnose the error, but he can read it, and he can tell us
about the specific error such-and-such. If programs fail silently, or
in a "graceful" way, you may end up with more "It doesn't
work"-user-feedback, leaving you to guess what's going wrong.



Sometimes you need error handling in production code, because nobody
can predict any and all circumstances, especially in application
development, where a 'handy' little app on the user end may be the
banana peel to a perfectly good, bug-free piece of software. For
example, Adobe Premiere and my custom tray-based character map app:
Premiere wouldn't even start. However, Premiere would display a
friendly message, "Sorry, gotta go, trying to save" instead of dumping
a Windows Illegal Operation on me.



Usually, though, I believe that try/catches are for bugzapping, not to
cover your ass when the code goes production. You're making the program
responsible for your laziness, or inability to grasp your program.
Which is entirely manageable for website code.

>I guess people who don't speak Dutch won't get it, but that is an odd choice for a >password. Does the admin have a thing for speed cameras?

From flTSP4#1 to Flitspaal... It requires some imagination on your part, though! By the way, is it me, or does this board have a rather high percentage of Dutch speaking people? Go low countries, go!

Yup, count me in. I guess the first poster has a thing for speed camera's himself [;)].

konijn:

Yup, count me in. I guess the first poster has a thing for speed camera's himself [;)].

I'd say no, but today was the very first time in the 7 years I've had
my driver's license that I "got caught". Annoying, since I usually
don't ever drive too fast. Somebody was pushing and I actually allowed
myself to get caught up in that.

Kippesoep:

konijn:

Yup, count me in. I guess the first poster has a thing for speed camera's himself [;)].

I'd say no, but today was the very first time in the 7 years I've had my driver's license that I "got caught". Annoying, since I usually don't ever drive too fast. Somebody was pushing and I actually allowed myself to get caught up in that.

Here in America, we've found something that general works for those situations:  the finger and the horn.  Failing that, there's always slamming on the brakes.  [:P]

Anonymous:

Although it pains me to search the hashtable twice to do one insert, this would have prevented this situation

Then don't search it twice, just do the following instead:

protected static void EnsureConnection(ConnectionProvider provider, string connectionString)
{
  lock(activeConnections)
  {
      if (!activeConnections.ContainsKey(connectionString))
      {
          DataConnection conn = CreateConnection(provider.ConnectionType); 
          activeConnections.Add(connectionString, conn);
      }
  }
}

Of course, what is really needed is a read-write lock, not just a mutual exclusion-lock as is currently used.



As a side-note, double-checked locking is BAD. in many cases. In Java
for example, it is not even guaranteed to work! (I don't know if CLR/IL
has been analysed sufficiently yet so any statments can be made about
C#)

Kippesoep:

konijn:

Yup, count me in. I guess the first poster has a thing for speed camera's himself [;)].

I'd say no, but today was the very first time in the 7 years I've had
my driver's license that I "got caught". Annoying, since I usually
don't ever drive too fast. Somebody was pushing and I actually allowed
myself to get caught up in that.

I can say the same. In the 27 years I have been driving I have been
caught  9 times  - once at 178 km/h  (111 mph for the
mph people).  Tends to happen when one drives long distnces
frequently - think something to do with the road getting boring and the
right foot getting heavy .... 

dhromed:

Usually, though, I believe that try/catches are for bugzapping, not to cover your ass when the code goes production. You're making the program responsible for your laziness, or inability to grasp your program. Which is entirely manageable for website code.

If this really is a "too many people logged in" error, then surely saying so is better than dumping a load of technical details that the user will never understand?  Using this type of thing (ignoring the security issue) will likely result in a lot of "It's dead! The world is ended!  It's saying duplicate keys, but I don't know what means!"-type feedback, usually answered by "someone else is already using the system, get them to log out."

Try/catches aren't for bugzapping, since you'll want as much info to be returned in the error as possible.  In fact, I sometimes /remove/ try/catches during bugzapping, so that it will give me the full info.  There are basically four cases with errors, IMHO:

1. An exception is thrown, but it's expected and hidden.  This isn't a problem, it's just that the program is using the exception to find out some info that can't be found out any other way.  For example, consider a system tray application that connects to an internet site (such as an auto-update thing for your virus checker).  Now, do you really want it to throw an exception every time it can't access the site?  No, you don't.


2. An exception is thrown, it indicates a problem, but the application knows how to deal with it, so it does so and the error is hidden from the user.  Supposing an application starts and tries to read its config file, which doesn't exist at that point.  It could error, but a better thing would be for it to create the file and populate it with default settings.


3. An exception is thrown, it indicates a problem, and this needs intervention from the user, so a sensible error is displayed.  Supposing you try to save a file, but the filename is invalid.  In this case, a short error message "invalid filename" is needed, not a long error string that you don't understand.


4. An exception is thrown, it indicates a problem, but no intervention from the user will be able to resolve it.  (Access violation, maybe).

Now, none of the first 3 cases should cause the application to crash, so a try/catch could and should be used.  The 4th case is the only type of error you shouldn't catch, but it should never happen anyway.

Anonymous:

By the way, is it me, or does this board have a rather high percentage of Dutch speaking people? Go low countries, go!

Nearly every forum of which I have ever been a part has had a high
"Dutch speaking" to "other people" ratio.  This has led me to
believe that Dutch speaking people created the internet and are now
watching/maintaining/controlling it.











I'm onto you...

Anonymous:

Kippesoep:

konijn:

Yup, count me in. I guess the first poster has a thing for speed camera's himself [;)].

I'd say no, but today was the very first time in the 7 years I've had my driver's license that I "got caught". Annoying, since I usually don't ever drive too fast. Somebody was pushing and I actually allowed myself to get caught up in that.

Here in America, we've found something that general works for those situations:  the finger and the horn.  Failing that, there's always slamming on the brakes.  [:P]

I guess if you want to be hit from behind.  I find that it's more effective to slow down gradually.  Then you really make them suffer.  That's actually what they taught me to do in driving school.  Since they are too close, you slow down to the point that they are at a safe following distance.  I always do this when I have some idiot riding my tail while there is an open lane to the left.

johnl:

phelyan:

Anonymous:

It seems nobody gets it: the user/pass-message is clearly coming from .NET, probably from the DB-driver (.NET JDBC equivalent). Yet another security short sight from Mickey-culture.

I hate to burst your 'randomly attacking Microsoft' bubble, but the user/pass is in the connectionString passed into the method. The HashMap is complaining about the duplicate key, not the value.

I think he was complaining about the username and password appearing in the error message. Of course, he's still talking rubbish - AFAIK, most database access engines, not just ADO.NET, will do that if you ask them to.  Besides, it's the programmers' mistake to include the whole string in the message, not Microsoft's.

It looks like the programmer decided to use the hash table to make sure he had no duplicate connection strings (by putting the value of the connection string in as the key, it'll throw an error if you get two the same).  This is a WTF in itself, since if those strings ever change then I don't think it'll allow you to, er, rename the key.  You'd have to remove it and add it again, I suppose.

Anyway, if you want to do that kind of thing, then you should have a catch handler which strips out all the sensitive information.

Verbose errors

The error message has nothing to do with databases and it is NOT a "stupid Microsoft feature" where the exception results in an error showing logon info. 

The error message is simply displaying the contents of the STRING that is causing the exception.   It just so happens that the programmer decided to store database connection information (including username/password info) in the string itself.

UncleMidriff:

Anonymous:

By the way, is it me, or does this board have a rather high percentage of Dutch speaking people? Go low countries, go!

Nearly every forum of which I have ever been a part has had a high "Dutch speaking" to "other people" ratio.  This has led me to believe that Dutch speaking people created the internet and are now watching/maintaining/controlling it.





I'm onto you...

Dutch is the VB6 of natural languages.

Seriously, have you heard those people speak?

 

 

 

 

(j/k)

dubwai:

Since they are too close, you slow down

I don't like it when I'm forced to slow down . . .

dubwai:

. . .while there is an open lane to the left.

but then, if I can pass, I'm not forced to slow down.



It is aggrivating when someone going too slowly on a road where passing
is not allowed.  Then you are a victim.  Sometimes, when I am
on such a road and someone behind me seems to want to go around, I will
move onto the shoulder and let them be on their way.  No reason to
make them conform to my speed (it is not my job to make other drivers go slow).  I wish others would do the same
for me.

loneprogrammer:

No reason to
make them conform to my speed (it is not my job to make other drivers go slow).  I wish others would do the same
for me.

Man, you should come drive round here. little wiggly mountain roads (with snow, as well, 4 months of the year) and hordes of people who are not safe off a motorway. It's a fucking disaster area, I tell ya. The only ones who pull over are the heavy goods boys, but they have to as they are actually not allowed to use the roads and don't want to get reported to the plod

Simon

Anonymous:

Kippesoep:

konijn:

Yup, count me in. I guess the first poster has a thing for speed camera's himself [;)].

I'd say no, but today was the very first time in the 7 years I've had my driver's license that I "got caught". Annoying, since I usually don't ever drive too fast. Somebody was pushing and I actually allowed myself to get caught up in that.

I can say the same. In the 27 years I have been driving I have been caught  9 times  - once at 178 km/h  (111 mph for the mph people).  Tends to happen when one drives long distnces frequently - think something to do with the road getting boring and the right foot getting heavy .... 

 

LOL, Flitspaal is the first thing I thought when seeing that password. Then the Dutch name of the victim... Can it really be a coincidence.. Just like LOL is also a Dutch word, meaning fun. Hmm..

Anyway never got caught by a flitspaal yet, after 9 years. I have pushers myself but braking hard and getting your car wrecked is not a solution.. Especially since it is illegal to brake for no reason.

Drak