Reputation: 331
I have a slight issue, I have a ASP.NET Webforms application. I'm sending over a url?id=X
were X
is my database index or id.
I have a C# class file to run my SQL connection and query. Here is the code:
public DataTable ViewProduct(string id)
{
try
{
string cmdStr = "SELECT * Products WHERE Idx_ProductId = " + id;
DBOps dbops = new DBOps();
DataTable vpTbl = dbops.RetrieveTable(cmdStr, ConfigurationManager.ConnectionStrings["MyDatabase"].ConnectionString);
return vpTbl;
}
catch (Exception e)
{
return null;
}
}
So as you can see my problem lies within string cmdStr = "SQL Query" + variable;
I'm passing over my index or id through the URL then requesting it and turning it into a string then using ViewProduct(productId)
.
I don't know what syntax or how to add the id into my C# string sql query. I've tried:
string cmdStr = "SELECT * Products WHERE Idx_ProductId = @0" + id;
string cmdStr = "SELECT * Products WHERE Idx_ProductId = {0}" + id;
also what I have currently to no avail.
Upvotes: 2
Views: 3434
Reputation: 21641
I was so sure this would be a duplicate of some canonical question about parameterized queries in C#, but apparently there isn't one (see this)!
You should parameterize your query - if you don't, you run the risk of a malicious piece of code injecting itself into your query. For example, if your current code could run against the database, it would be trivial to make that code do something like this:
// string id = "1 OR 1=1"
"SELECT * Products WHERE Idx_ProductId = 1 OR 1=1" // will return all product rows
// string id = "NULL; SELECT * FROM UserPasswords" - return contents of another table
// string id = "NULL; DROP TABLE Products" - uh oh
// etc....
ADO.NET provides very simple functionality to parameterize your queries, and your DBOps
class most assuredly is not using it (you're passing in a built up command string). Instead you should do something like this:
public DataTable ViewProduct(string id)
{
try
{
string connStr = ConfigurationManager.ConnectionStrings["MyDatabase"].ConnectionString;
using (SqlConnection conn = new SqlConnection(connStr))
{
conn.Open();
using (SqlCommand cmd = conn.CreateCommand())
{
// @id is very important here!
// this should really be refactored - SELECT * is a bad idea
// someone might add or remove a column you expect, or change the order of columns at some point
cmd.CommandText = "SELECT * Products WHERE Idx_ProductId = @id";
// this will properly escape/prevent malicious versions of id
// use the correct type - if it's int, SqlDbType.Int, etc.
cmd.Parameters.Add("@id", SqlDbType.Varchar).Value = id;
using (SqlDataReader reader = cmd.ExecuteReader())
{
DataTable vpTbl = new DataTable();
vpTbl.Load(reader);
return vpTbl;
}
}
}
}
catch (Exception e)
{
// do some meaningful logging, possibly "throw;" exception - don't just return null!
// callers won't know why null got returned - because there are no rows? because the connection couldn't be made to the database? because of something else?
}
}
Now, if someone tries to pass "NULL; SELECT * FROM SensitiveData", it will be properly parameterized. ADO.NET/Sql Server will convert this to:
DECLARE @id VARCHAR(100) = 'NULL; SELECT * FROM SensitiveData';
SELECT * FROM PRoducts WHERE Idx_ProductId = @id;
which will return no results (unless you have a Idx_ProductId
that actually is that string) instead of returning the results of the second SELECT
.
Some additional reading:
Upvotes: 4
Reputation: 358
What type Products.Idx_ProductId is?
Probably it is string, than you need to use quotes: "... = '" + id.Trim() + "'";
Upvotes: 0