[Last Call] Learn how to a build a cloud-first strategyRegister Now

x
?
Solved

Will this code prevent SQL Injection as well as optimize my SQL Statements automatically?

Posted on 2006-05-03
12
Medium Priority
?
414 Views
Last Modified: 2007-12-19
String insertSQL = "Insert into Stores (stor_id, stor_name, stor_address, city, state, zip) values ('" + storeid.Value + "', '" + storename.Value + "', '" + address.Value + "', '" + city.Value + "', '" + state.Value + "', '" + zip.Value + "')";

SqlConnection objConnect = new SqlConnection("server=(local);database=pubs;uid=sa;pwd=;");

SqlHelper.ExecuteNonQuery (objConnect, CommandType.Text, insertSQL);
 
0
Comment
Question by:tatton777
  • 3
  • 2
  • 2
  • +3
12 Comments
 
LVL 10

Expert Comment

by:smegghead
ID: 16601525
No, it's not protected - the best way to avoid injection is to use sqlparameters rather than concatenating strings.

If stroreid.Value contains the following text

xxx','xxx','xxx','xxx','xxx','xxx') \r\n go \r\n delete from [Stores] \r\n go

this would wipe out the Stores table.

Smg.
0
 

Expert Comment

by:bob20062006
ID: 16601675
the best security is with make sored procedure and call it bt pass  parameters
0
 
LVL 6

Expert Comment

by:PoeticAudio
ID: 16601679
AFAIK it's safer to use parameters. Here is a little example (this should give you an idea, but I don't have VS in front of me, so there may be some syntax errors)

string insertSQL = "INSERT INTO Stores (StoreName) VALUES (@StoreName)

SqlConnection conn = new SqlConnection(connectionString");
SqlCommand cmd = new SqlCommand(insertSQL, conn);
cmd.Parameters.Add("@StoreName", storename.Value);

conn.Open();
cmd.ExecuteNonQuery();
conn.Close();


I may have missed something, this was just off the top of my head (don't have VS in front of me).
0
VIDEO: THE CONCERTO CLOUD FOR HEALTHCARE

Modern healthcare requires a modern cloud. View this brief video to understand how the Concerto Cloud for Healthcare can help your organization.

 

Expert Comment

by:bob20062006
ID: 16601697
yes i think PoeticAudio  is the best programing solution to privent sql injection
0
 
LVL 6

Expert Comment

by:PoeticAudio
ID: 16601728
Oops, found a couple... here's a better version...

string connectionString = "server=(local);database=pubs;uid=sa;pwd=;";
string insertSQL = "INSERT INTO Stores (StoreName) VALUES (@StoreName)"

SqlConnection conn = new SqlConnection(connectionString);
SqlCommand cmd = new SqlCommand(insertSQL, conn);
cmd.Parameters.Add("@StoreName", storename.Value);

conn.Open();
cmd.ExecuteNonQuery();
conn.Close();
0
 
LVL 12

Expert Comment

by:AGBrown
ID: 16601818
I'm going to re-enforce what Smg and PoeticAudio say (and qualify bob's sproc statement) with the following:
1) The only way of preventing sql injection is by using named parameters in a statement
2) Creating dynamic sql without named parameters is still possible inside a stored procedure, and will _not_ prevent sql injection

Therefore named parameters in a statement, as PoeticAudio states, or stored procedures that contain explicit SQL statements, and not dynamic SQL, will prevent SQL injection.

The choice for Stored procedures over dynamic SQL with named parameters therefore comes down to where you sit on seperation of concerns, and your interpretation of the definition of loose coupling in your database and data access layers (I won't state my opinion on this). That notwithstanding, the advantage for storing SQL statements in the database in the form of explicit SQL inside stored procedures, is pre-compilation and performance. Arguably, for longer statements, it is also readability of code.

Hth,

Andy
0
 
LVL 1

Author Comment

by:tatton777
ID: 16601891
If know how to write some crappy little stored procedures but not long complex ones. If I take the time, the learn how to write the stored procedures that I need. Will the sqlHelper become a viable, useful tool for me to write secure and optimized code. IF NOT, how would I used the stored procedures. Still learning here, sorry my questions are a little lame.
0
 
LVL 12

Expert Comment

by:AGBrown
ID: 16601953
There are a lot of advantages to using stored procedures. For example:
1) Not having to re-release your website when you find an error in your SQL.
2) Optimising longer SQL statements for which you have several options.
3) An extra layer of security. You can enforce, through the database, that the account that your application runs under only has execute permissions on certain stored procedures that it needs, and not full control over all objects in the database. This is arguably possible without using stored procedures, but it is a lot easier with stored procedures.
4) Easily improving on your SQL without re-compiling and releasing your application

OOP evangelists may even go so far as to say that you are implementing a definite declarative interface through which to carry out a particular piece of functionality in the database. Be that as it may, or may not be, I solely use stored procedures. I also control access to them by defining a "role" in the database that has execute permissions on the stored procedures a particular application needs access to, and putting the user account that runs that application into that role - this makes server deployment secure and very easy.

I also use the DAAB to execute the stored procedures (or rather, a considerably adapted version of it). Which version of the data access application block are you using? Are you using the Enterprise Library, and are you using it in .NET 1.1 or 2.0? Actually, given that you are talking about the SqlHelper you might be using DAB 2.0 - is that the case?

Either way, the SqlHelper takes a lot of the effort out of writing code to have conversations with a database. It doesn't necessitate you using stored procedures, you can still use parameterised sql, but that takes a lot of the point out of using the daab. The SqlHelper will not necessarily ensure you write "secure and optimized code". As I said, you can still open yourself up to SQL injection by writing dynamic, nonparameterised SQL in a stored procedure. It does, however, take a lot of the effort out of accessing the database, and more importantly it puts the code that manages the database conversations in one place, which makes it a lot easier to maintain and improve on. Yes, you can improve on the sqlhelper and the DAB in the Enterprise Library by making it better fit your scenarios. Thankfully the out-of-the-box functionality is so good that you won't need to consider that until a lot later on.

In terms of optimisation - the original SqlHelper was written to perform "within 5% of ADO.NET" (if i remember correctly). I'm not sure of the definition of 5%, but that seems like an acceptable hit to take when you are talking about reuseability, maintainability and reduction of boilerplate code.

Andy
0
 
LVL 1

Author Comment

by:tatton777
ID: 16602379
So the best route for me to take is to learn to write Stored Procedures and combine them with DAAB. Can someone point me to a resource for writing complex stored procedures and then using them in conjunction with DAAB.
0
 
LVL 19

Expert Comment

by:Fahad Mukhtar
ID: 16602974
Here are some best practices... very good article
http://msdn.microsoft.com/msdnmag/issues/04/09/SQLInjection/
0
 
LVL 12

Accepted Solution

by:
AGBrown earned 2000 total points
ID: 16606506
I wouldn't worry too much about the complexity of your stored procedures; whether a stored procedure is simple, or complex the end result will be the same in terms of using the sp with the DAB.

The basic things to know how to do are:
1) Using input parameters
2) Using output parameters (declare as @varname <datatype> OUTPUT in your sp, and state ParameterDirection for corresponding parameter in code)
3) Using ExecuteNonQuery for sps that don't return a resultset
4) Using ExecuteNonQuery with output parameters, or ExecuteScalar, to get values back out of a sproc that doesn't return a resultset
5) Using ExecuteReader or ExecuteDataset to get resultsets out (and using output parameters to get values out)

The best resource for using the DAAB is the documentation that comes with it. In terms of writing stored procedures, basically you don't need to know a lot more than you do already. If you can understand the SQL below, then you know all you need to know in terms of writing basic sprocs. After that, the help files with SQL server are your best technical resource.

CREATE PROC MyProc
( @intInputParam int, @intOutputParam int OUTPUT )
AS

-- insert into some table
INSERT INTO MyTableWithIdentity
    (valueColumn)
VALUES
    ('test')

SET @intOutputParam = Scope_Identity()

-- now get a resultset
SELECT *
FROM MyOtherRelatedTable
WHERE AColumn = 'Something'

GO
0
 
LVL 10

Expert Comment

by:smegghead
ID: 16606702
another quick solution to this, if you don't want to write stored procedures, is to double up the single quotes in your parameters before concatenating them to the string. This prevents sql injection.

for example, my injection example would be converted to ..

xxx'',''xxx'',''xxx'',''xxx'',''xxx'',''xxx'') \r\n go \r\n delete from [Stores] \r\n go

use MyPar=MyPar.Replace("'","''"); // there is one single quote in the first string, and two in the second... if you're having trouble focusing.

this way, the whole string would be treated as a parameter, not letting the 'injector' escape the quotes.

However, SP's with parameters are the long term solution to this.

Regards
Smg.
0

Featured Post

Free Tool: Port Scanner

Check which ports are open to the outside world. Helps make sure that your firewall rules are working as intended.

One of a set of tools we are providing to everyone as a way of saying thank you for being a part of the community.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Introduction Hi all and welcome to my first article on Experts Exchange. A while ago, someone asked me if i could do some tutorials on object oriented programming. I decided to do them on C#. Now you may ask me, why's that? Well, one of the re…
This article aims to explain the working of CircularLogArchiver. This tool was designed to solve the buildup of log file in cases where systems do not support circular logging or where circular logging is not enabled
Is your data getting by on basic protection measures? In today’s climate of debilitating malware and ransomware—like WannaCry—that may not be enough. You need to establish more than basics, like a recovery plan that protects both data and endpoints.…
With just a little bit of  SQL and VBA, many doors open to cool things like synchronize a list box to display data relevant to other information on a form.  If you have never written code or looked at an SQL statement before, no problem! ...  give i…
Suggested Courses

831 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question