String.Format???
-
In many cases a stored procedure would offer some benefits. The real horror here is that the parameters were added with string formatting. Command parameters would have been a far better choice. This way, depending on where the parameters come from, the door is open for SQL injection attacks.
A while ago he asked me what he should have printed on my business cards. I said 'Wizard'. I read books which nobody else understand. Then I do something which nobody understands. After that the computer does something which nobody understands. When asked, I say things about the results which nobody understand. But everybody expects miracles from me on a regular basis. Looks to me like the classical definition of a wizard.
-
Using SQL in code is generally considered bad practice. It can raise maintenance issues and can be exposed to SQL injection attacks to name just a few reasons.
Admittedly a SP would be much a nicer solution, but in real life you may have to work with databases where you are nowhere near getting authorized to implement a SP. Think of implementing a reporting system for at large financial institution as a consultant. What do you think their reply would be if you came saying "I need a dozen new stored procedures in your central DB2-database"? The polite answers would be something along the lines of "I'm sorry but that won't be possible", "Are you quite sure this is needed?" etc, etc. The impolite answer would be to find someone else to do the job.
-
Admittedly a SP would be much a nicer solution, but in real life you may have to work with databases where you are nowhere near getting authorized to implement a SP. Think of implementing a reporting system for at large financial institution as a consultant. What do you think their reply would be if you came saying "I need a dozen new stored procedures in your central DB2-database"? The polite answers would be something along the lines of "I'm sorry but that won't be possible", "Are you quite sure this is needed?" etc, etc. The impolite answer would be to find someone else to do the job.
I agree, that might be the case, but the fact still remains (as CDP1802 noted), that having SQL statement in code formatted with parameters that might come from e.g. UI text-boxes, represents great vulnerability to SQL injection attacks. Otherwise I understand that sometimes there is no other way, nevertheless in-code SQL can be used wisely or not.
-
In many cases a stored procedure would offer some benefits. The real horror here is that the parameters were added with string formatting. Command parameters would have been a far better choice. This way, depending on where the parameters come from, the door is open for SQL injection attacks.
A while ago he asked me what he should have printed on my business cards. I said 'Wizard'. I read books which nobody else understand. Then I do something which nobody understands. After that the computer does something which nobody understands. When asked, I say things about the results which nobody understand. But everybody expects miracles from me on a regular basis. Looks to me like the classical definition of a wizard.
I think in that case you can use a in code SQL, but using SQL parameters. Then, in you SQL command you add the parameters. Something like this:
string query = "select * from table where column = @value";
SqlCommand cmd = new SqlCommand(query);
cmd.Parameters.Add("@value", textBox1.Text);This SQL statments are cached (so if you execute it in short time intervals the SQL plan will be computed) and aren't vunerable to SQL injection.
-
In many cases a stored procedure would offer some benefits. The real horror here is that the parameters were added with string formatting. Command parameters would have been a far better choice. This way, depending on where the parameters come from, the door is open for SQL injection attacks.
A while ago he asked me what he should have printed on my business cards. I said 'Wizard'. I read books which nobody else understand. Then I do something which nobody understands. After that the computer does something which nobody understands. When asked, I say things about the results which nobody understand. But everybody expects miracles from me on a regular basis. Looks to me like the classical definition of a wizard.
How would you add the parameter for a query like:
SELECT * FROM Table WHERE ID IN (123,124,125);? Lists are a bit tougher to handle in a proper way..."When did ignorance become a point of view" - Dilbert
-
Using SQL in code is generally considered bad practice. It can raise maintenance issues and can be exposed to SQL injection attacks to name just a few reasons.
tinko101 wrote:
SQL injection attacks
That's only due to this particular command not using parameters.
tinko101 wrote:
raise maintenance issues
Stored procedures raise maintenance issues too. And how will you execute your stored procedure from code without having SQL in your code?
-
How would you add the parameter for a query like:
SELECT * FROM Table WHERE ID IN (123,124,125);? Lists are a bit tougher to handle in a proper way..."When did ignorance become a point of view" - Dilbert
Personally I wouldn't. I wouldn't use
INat all, I'd find a way to have a table on which toJOINinstead. The statement you present is a symptom of a poorly implemented system. -
It isn't; pay no attention to the stored procedure fan boys.
-
I agree, that might be the case, but the fact still remains (as CDP1802 noted), that having SQL statement in code formatted with parameters that might come from e.g. UI text-boxes, represents great vulnerability to SQL injection attacks. Otherwise I understand that sometimes there is no other way, nevertheless in-code SQL can be used wisely or not.
tinko101 wrote:
in-code SQL can be used wisely or not
Exactly.
tinko101 wrote:
SQL injection attacks.
Parameterization handles that regardless of where the SQL statement is stored.
modified on Friday, July 9, 2010 12:02 PM
-
It isn't; pay no attention to the stored procedure fan boys.
-
While we are at it, could someone please explain to me why inline SQL might be more vulnerable to SQL injection than a stored proc. Having thought about it all afternoon, i must admit that i just can't see it. Am i blind?
It really isn't. However, when using stored procedures, you are pretty much forced to use parameters. When using embedded SQL, you have the option, and they who don't know about parameters or are too lazy to bother, wind up with bad code. Basically, whichever way you store your SQL statements, use parameters. A properly-written Data Access Layer will hide the details anyway.
-
It really isn't. However, when using stored procedures, you are pretty much forced to use parameters. When using embedded SQL, you have the option, and they who don't know about parameters or are too lazy to bother, wind up with bad code. Basically, whichever way you store your SQL statements, use parameters. A properly-written Data Access Layer will hide the details anyway.
Here's a good article about SQL Injection: http://www.mikesdotnetting.com/Article/113/Preventing-SQL-Injection-in-ASP.NET[^]
Steve Wellens
-
Here's a good article about SQL Injection: http://www.mikesdotnetting.com/Article/113/Preventing-SQL-Injection-in-ASP.NET[^]
Steve Wellens
Yes, that's good.
-
Personally I wouldn't. I wouldn't use
INat all, I'd find a way to have a table on which toJOINinstead. The statement you present is a symptom of a poorly implemented system.There was a thread on here recently discussing this, comparing IN, EXISTS and JOIN. I can't find it now, but if I remember correctly there was a link on there to a blog from one of the SQL Server tech-heads that explained why and how the three are not interchangeable.
-
While working with a client to help them clean up their code I found this little gem. They absolutely never knew string.Format existed :wtf:
string sql = "select * from table where id={0} and date={1}";
string cmdText = sql.replace("{0}", id.Tostring())
.replace("{1}", DateTime.Now.ToShortDateString());
I know the language. I've read a book. - _Madmatt
This looks like a direct port from MFC C++ codebase to .NET and C# from someone not quite familiar with the C#.
The narrow specialist in the broad sense of the word is a complete idiot in the narrow sense of the word. Advertise here – minimum three posts per day are guaranteed.
-
There was a thread on here recently discussing this, comparing IN, EXISTS and JOIN. I can't find it now, but if I remember correctly there was a link on there to a blog from one of the SQL Server tech-heads that explained why and how the three are not interchangeable.
That might be an interesting read; I'll take a look to see if I can find it. On the other hand, JOIN can do what IN and EXISTS can do, but IN and EXISTS can't do what JOIN does. P.S. I just searched the general database forum back to May 1 and didn't find it.
modified on Thursday, August 5, 2010 12:02 AM
-
Personally I wouldn't. I wouldn't use
INat all, I'd find a way to have a table on which toJOINinstead. The statement you present is a symptom of a poorly implemented system. -
How would you add the parameter for a query like:
SELECT * FROM Table WHERE ID IN (123,124,125);? Lists are a bit tougher to handle in a proper way..."When did ignorance become a point of view" - Dilbert
I would imagine this would work:
string query = "select * from table where ID in (@value1, @value2, @value3)";
SqlCommand cmd = new SqlCommand(query);
cmd.Parameters.Add("@value1", 123);
cmd.Parameters.Add("@value2", 124);
cmd.Parameters.Add("@value3", 125);If not, this surely would:
string query = "select * from table where (ID = @value1 or ID = @value2 or ID = @value3)";
SqlCommand cmd = new SqlCommand(query);
cmd.Parameters.Add("@value1", 123);
cmd.Parameters.Add("@value2", 124);
cmd.Parameters.Add("@value3", 125); -
Using an SQL IN-clause is definetely not a design flaw. Forcing everything into JOIN's is on the other hand an odd self-imposed hinderence.
While it is true that one should "use the right tool for the right job", I have never used EXISTS (I have an Oracle background), and I have not used IN/NOT IN for many years, and never with SQL Server. JOIN tends to scale better -- you may have an IN, EXISTS, or even a BETWEEN that has to be converted to a JOIN as the project becomes more complex; using a JOIN to begin with eases such maintenance. JOIN allows you to configure a system by maintaining a table rather than modifying the code. As with Jörgen's post, an IN with hard-coded values I especially discourage; they reek of the "magic numbers" code smell. A subquery on some table would at least improve maintainability.
-
I agree, that might be the case, but the fact still remains (as CDP1802 noted), that having SQL statement in code formatted with parameters that might come from e.g. UI text-boxes, represents great vulnerability to SQL injection attacks. Otherwise I understand that sometimes there is no other way, nevertheless in-code SQL can be used wisely or not.
Did you look at the code in the OP? You can't do SQL injection if your parameter data is strongly typed Int32 and DateTime values. If it was a string that's a different story, but if you are doing extra code to make sure no one is slipping SQL keywords into your ints then you are wasting a lot of time way overarchitecting.