|
All of your replies, disappointingly, contain a major security flaw. You should never inject values into a SQL string when you can use a parameter instead. For more information see SQL Injection Attacks and Tips on How to Prevent Them[^]
You may want to re-write your code to resemble this:
string SQLString = "SELECT * FROM Costs WHERE Costs.PartID = @PartID";
SqlCommand cmd = new SqlCommand();
cmd.Connection = myConnection;
cmd.CommandText = SQLString;
cmd.Parameters.Add("@PartID", strPartNumberInput);
If Costs.PartID is an int column then you'll have to convert the strPartNumberInput into an integer first: Convert.ToInt32(strPaetNumberInput)
Does this help?
"On two occasions, I have been asked [by members of Parliament], 'Pray, Mr. Babbage, if you put into the machine wrong figures, will the right answers come out?' I am not able to rightly apprehend the kind of confusion of ideas that could provoke such a question."
--Charles Babbage (1791-1871)
My: Website | Blog
|
|
|
|
|
I have not had a chance to read the article you referenced, but I am wondering why injecting values into the string is considered a security risk?
..big thanks to all who have replied to my question!
|
|
|
|
|
Because if you inject strings into the SQL, especially ones that come straight from the user interface, then an attacker can produce malformed SQL and gain access to your system. (Where do you live? I can come and do one of my SQL Injection Attack presentations in your town if you want a real live demonstration where I compromise a SQL Server into divulging the inner most secrets of the server it is running on. And I mean the whole server, not just the SQL Server process.*)
Lets say you have a simple bit of SQL like this:
cmd.CommandText = "SELECT * FROM Products where Name = '"+txtSearch.Text+"'";
What happens if the user types in the following?
'; DELETE FROM Products; --
The whole string becomes:
SELECT * FROM Products where Name = ''; DELETE FROM Products; --
That will return a dataset back to the application, which is what it expects, and then deletes all the products from the database. When the next customer comes to the website what is it going to show when there are no products in the database?
Okay - there may be some constraints on the table (foreign key constraints) that don't permit the rows to be deleted. How about something equally damaging to the company. Let's set their entire inventory to a penny!
The mallicious user then types:
'; UPDATE Products SET Price = 0.01; --
The word will quickly spread around the internet and the company will soon be out of business or have a huge number of very pissed off customers.
If you don't secure your system the possibilities for attack are endless.
* The demonstration is done on a server box that I own. Performing a SQL Injection Attack on a system without the permission of the system owner is a breach of the 1990 Misue of Computers Act and can carry a penalty of 5 years in jail.
"On two occasions, I have been asked [by members of Parliament], 'Pray, Mr. Babbage, if you put into the machine wrong figures, will the right answers come out?' I am not able to rightly apprehend the kind of confusion of ideas that could provoke such a question."
--Charles Babbage (1791-1871)
My: Website | Blog
|
|
|
|
|
Unfortunately I live in the boring state of South Dakota in the United States which would be a bit far for you to travel....
This is for an intranet site that only other programmers will be accessing. Apparently the company does not trust other users to update product information. Therefore security right now is not a concern to my boss but I'd like to get the security set in anticipation that non-programmers could use the application.
I skimmed the article and read your post. Very interesting stuff. I hope everyone takes the time to read it.
Thanks again!
|
|
|
|
|
Colin--I entered the code you provided...
private void btnSearchPartNumber_Click(object sender, System.EventArgs e)
{
strPartNumberInput = txtPartNumber.Text;
Convert.ToInt32(strPartNumberInput);
string SQLString = "Select * FROM Costs where Costs.PartID = @PartID";
SqlCommand cmd = new SqlCommand();
cmd.Connection = myConnection;
cmd.CommandText = SQLString;
cmd.Parameters.Add = ("@PartID", strPartNumberInput);
//Call and build grid
BindGrid(strConnectSQL, SQLString, DataGrid1);
}
However, I am getting a compiling error where I bolded the strPartNumberInput. I'm getting 'expected ;' What am I missing here?
|
|
|
|
|
Add is a method, not a property.
Use this instead...
cmd.Parameters.Add("@PartID", strPartNumberInput);
|
|
|
|
|
Two changes:
the line:
Convert.ToInt32(strPartNumberInput);
becomes
int partNumber = Convert.ToInt32(strPartNumberInput);
And the line
cmd.Parameters.Add = ("@PartID", strPartNumberInput);
becomes
cmd.Parameters.Add("@PartID", partNumber);
leckey wrote: BindGrid(strConnectSQL, SQLString, DataGrid1);
I'm not sure what that does (I'm guessing it is a method you created), but you need to send the command, not the string (remember the string now contains a parameter and the SQL Server needs to know what that means, which is what the cmd.Parameters.Add(...) does) And the method will have to be updated to use the command rather than the string.
I'm also guessing that strConnectSQL is your connection string - If so then you'll have to replace my reference to myConnection with your connection string.
"On two occasions, I have been asked [by members of Parliament], 'Pray, Mr. Babbage, if you put into the machine wrong figures, will the right answers come out?' I am not able to rightly apprehend the kind of confusion of ideas that could provoke such a question."
--Charles Babbage (1791-1871)
My: Website | Blog
|
|
|
|
|
Okay...here is all the code...
using System;
using System.Collections;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Web;
using System.Web.SessionState;
using System.Web.UI;
using System.Web.UI.WebControls;
using System.Web.UI.HtmlControls;
using System.Data.SqlClient;
using System.Configuration;
namespace WebBasedPartsDB
{
///
/// Summary description for WebForm1.
///
public class WebForm1 : System.Web.UI.Page
{
protected System.Web.UI.WebControls.Label lblPartNumber;
protected System.Web.UI.WebControls.Button btnSearchPartNumber;
protected System.Web.UI.WebControls.DataGrid DataGrid1;
protected System.Web.UI.WebControls.TextBox txtPartNumber;
protected string strPartNumberInput;
//Get the SQL connection string from the web.config file
public String strConnectSQL = (ConfigurationSettings.AppSettings["ConnectionString"]);
private void Page_Load(object sender, System.EventArgs e)
{
// Put user code to initialize the page here
}
#region Web Form Designer generated code
override protected void OnInit(EventArgs e)
{
//
// CODEGEN: This call is required by the ASP.NET Web Form Designer.
//
InitializeComponent();
base.OnInit(e);
}
///
/// Required method for Designer support - do not modify
/// the contents of this method with the code editor.
///
private void InitializeComponent()
{
this.btnSearchPartNumber.Click += new System.EventHandler(this.btnSearchPartNumber_Click);
this.Load += new System.EventHandler(this.Page_Load);
}
#endregion
private void btnSearchPartNumber_Click(object sender, System.EventArgs e)
{
strPartNumberInput = txtPartNumber.Text;
int partNumber = Convert.ToInt32(strPartNumberInput);
string SQLString = "Select * FROM Costs where Costs.PartID = @PartID";
SqlCommand cmd = new SqlCommand();
cmd.Connection = strConnectSQL;
cmd.CommandText = SQLString;
cmd.Parameters.Add ("@PartID", partNumber);
//Pass the command, not the string
BindGrid (strConnectSQL, SQLString, DataGrid1 );
//BindGrid(strConnectSQL, cmd, DataGrid1);
}
//**********************************************************
//BindData()
//**********************************************************
private void BindGrid (string DBconnectString, string sqlCommand, System.Web.UI.WebControls.DataGrid DGrid)
{
// create data connection
SqlConnection conn = new SqlConnection(DBconnectString);
// Call SP from db
SqlCommand command = new SqlCommand(sqlCommand, conn);
// create data adapter
SqlDataAdapter adapter = new SqlDataAdapter(command);
// create and fill dataset
DataSet ds = new DataSet();
adapter.Fill(ds);
// fill and bind data to Datagrid
DGrid.DataSource = ds;
DGrid.DataBind();
// Close Connection
conn.Close();
}
}
}
The only compile error I'm getting is 'Cannot implicitly convert type 'string' to 'System.Data.SqlClient.SqlConnection' where I highlighted....
Sorry, folks. I'm a project manager who has been thrown into programming. I'm sure this seems simple to you!
|
|
|
|
|
Connection expects to be assigned a SqlConnection object, not a string. Use this instead:
cmd.Connection = new SqlConnection( strConnectSQL );
Also, you should be sure to call Dispose on cmd at the end of the method, which releases the resources used by that command object.
Josh
|
|
|
|
|
I've read you can Close and Dispose. Is Dispose better?
|
|
|
|
|
As far as I know they do the same thing. Usually if a class implements both a Dispose and Close method, one of them just calls the other.
Josh
-- modified at 15:40 Wednesday 7th June, 2006
Oops, SqlCommand does not have a Close method. SqlConnection has a Close method. You should call Dispose on the SqlCommand.
|
|
|
|
|
Okay, so the part number the user enters is an alpha-numeric id that never really gets used again. The SQL table Parts has two fields: Parts.PartNumber (which is what the user enters) and Parts.Id (which is the actual field that I have to match up with in Costs.PartID). Do I basically have to repeat everything to get that field before joining with costs? What I mean is, if I do Colin's reference:
SqlCommand cmd = new SqlCommand();<br />
cmd.Connection = new SqlConnection(strConnectSQL);<br />
cmd.CommandText = SQLString;<br />
cmd.Parameters.Add ("@PartID", partNumber);
And I want to do another @TruePartId (or something like that) do I have to go through the SqlCommand cmd = new SqlCommand again?
I'm so sorry...my head is swimming...thank you guys so much for explaining this.
|
|
|
|
|
Maybe it's me, but I'm not clear on what you're asking. You might want to elaborate a little bit on the situation.
leckey wrote: do I have to go through the SqlCommand cmd = new SqlCommand again?
 Copy-and-paste helps alleviate the pain.
Josh
|
|
|
|
|
Basically my code is missing a step. Right now the user enters the Part Number (Parts.PartNumber which is a varchar). Right now the code says, okay give me info from costs where Costs.PartID matches Parts.PartNumber. But it's never going to match up because I missed a step.
For example, product C15690 has an ID of 5499. This is what I need to match.
Right now the code goes like this....
Costs.PartID = Parts.PartNumber
Should be...
First get Parts.ID based on Parts.PartNumber (missing step)
Costs.PartID = Parts.ID
private void btnSearchPartNumber_Click(object sender, System.EventArgs e)<br />
{<br />
strPartNumberInput = txtPartNumber.Text;<br />
int partNumber = Convert.ToInt32(strPartNumberInput);
string SQLString = "Select * FROM Costs where Costs.PartID = @PartID"; <br />
SqlCommand cmd = new SqlCommand();<br />
cmd.Connection = new SqlConnection(strConnectSQL);<br />
cmd.CommandText = SQLString;<br />
cmd.Parameters.Add ("@PartID", partNumber);<br />
BindGrid (strConnectSQL, SQLString, DataGrid1 );<br />
}
Oh, I hope this explains it!
|
|
|
|
|
Your SQL query needs to join the Costs and Parts table on the part ID. You will only need one call to the database. Something like:
SELECT *
FROM Costs c INNER JOIN Parts p On c.PartID = p.ID
WHERE p.PartNumber = @PartNumber
Josh
|
|
|
|
|
private void btnSearchPartNumber_Click(object sender, System.EventArgs e)<br />
{<br />
strPartNumberInput = txtPartNumber.Text;<br />
int partNumber = Convert.ToInt32(strPartNumberInput);<br />
string SQLString = "Select * FROM Costs c INNER JOIN Parts p ON c.PartID = p.Id where p.PartNumber = @PartID"; <br />
SqlCommand cmd = new SqlCommand();<br />
cmd.Connection = new SqlConnection(strConnectSQL);<br />
cmd.CommandText = SQLString;<br />
cmd.Parameters.Add ("@PartID", partNumber);<br />
BindGrid (strConnectSQL, SQLString, DataGrid1 );<br />
}
Okay, when I try to load in the brower I get the error that I need to declare the variable '@PartID.'Since this is a reference with the @ sysmbol do I declare it differently or just declare it like a normal variable?
...thanks again for your help. I really appreciate you taking your own time to help me.
|
|
|
|
|
It looks like you need to pass cmd into the BindGrid method. As of now, the parameter you added to cmd is not being used because cmd is not being used.
josh
|
|
|
|
|
I had actually tried that before based on Colin's comments...
private void btnSearchPartNumber_Click(object sender, System.EventArgs e)<br />
{<br />
strPartNumberInput = txtPartNumber.Text;<br />
int partNumber = Convert.ToInt32(strPartNumberInput);<br />
string SQLString = "Select * FROM Costs c INNER JOIN Parts p ON c.PartID = p.Id where p.PartNumber = @PartID"; <br />
SqlCommand cmd = new SqlCommand();<br />
cmd.Connection = new SqlConnection(strConnectSQL);<br />
cmd.CommandText = SQLString;<br />
cmd.Parameters.Add ("@PartID", partNumber);<br />
BindGrid (strConnectSQL, cmd, DataGrid1);<br />
}
When I compile I get these errors:
The best overloaded method match for 'WebBasedPartsDB.WebForm1.BindGrid(string, string, System.Web.UI.WebControls.DataGrid)' has some invalid arguments
Argument '2': cannot convert from 'System.Data.SqlClient.SqlCommand' to 'string'
I don't understand how 'cmd' is invalid? Does the second error have something to do with the convert to int32?
|
|
|
|
|
cmd is not a string, which is why the compiler won't let you treat it as one. Change the signature of the BindGrid method so that it accepts a SqlCommand.
This will be my last reply on this thread.
Good luck,
Josh
|
|
|
|
|
Continuing from Josh's last comment:
Call BindGrid like this:
BindGrid(cmd, DataGrid1);
private void BindGrid (SqlCommand cmd, System.Web.UI.WebControls.DataGrid DGrid)
{
SqlDataAdapter adapter = new SqlDataAdapter(command);
DataSet ds = new DataSet();
adapter.Fill(ds);
DGrid.DataSource = ds;
DGrid.DataBind();
conn.Close();
}
All that was done here was the the first two parameters, the connection string and the command string, were replaced with one SqlCommand object.
The call to BindGrid similarly replaces the strings with a SqlCommand object.
"On two occasions, I have been asked [by members of Parliament], 'Pray, Mr. Babbage, if you put into the machine wrong figures, will the right answers come out?' I am not able to rightly apprehend the kind of confusion of ideas that could provoke such a question."
--Charles Babbage (1791-1871)
My: Website | Blog
|
|
|
|
|
Since 'conn' is not in BindGrid, I'm not sure what to close. I played around with the code, but it's not compiling.
|
|
|
|
|
leckey wrote: Since 'conn' is not in BindGrid, I'm not sure what to close
But it is in cmd - see SqlCommand.Connection[^]
"On two occasions, I have been asked [by members of Parliament], 'Pray, Mr. Babbage, if you put into the machine wrong figures, will the right answers come out?' I am not able to rightly apprehend the kind of confusion of ideas that could provoke such a question."
--Charles Babbage (1791-1871)
My: Website | Blog
|
|
|
|
|
RadioButtonList and CheckBoxList both share certain common properties
(e.g.
RepeatColumns
RepeatDirection
RepeatLayout )
However these properties are not inherited from a common ancestor.
Is there any neat way I can have 1 common function accepting either a RadioButtonList or CheckBoxList and use these properties within the function method without having to check the type and then casting to 1 or the other?
|
|
|
|
|
You could do it with reflection. But that's about it.
|
|
|
|
|
You could create your own class which wraps either a RadioButtonList or a CheckBoxList and exposes those common properties. It still wouldn't be an entirely statisfactory solution, but at least it would hide all the messy type checking and casting.
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
