|
Gerald Schwab wrote: string SQLString = "Select * FROM Costs WHERE costs.PartID = " + strPartNumberInput;
Your solution is susceptable to a SQL Injection Attack. Please learn how to defend against these attacks as they could compromise your systems. For more information see SQL Injection Attacks and Tips on How to Prevent Them[^]
"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
|
|
|
|
|
Uhh, he asked what he should concatenate to the end of the SQLString string variable in order to generate a SQL statement that would filter by Parts.ID. He didn't ask for a "solution". So, I didn't provide him with "my" solution, I simply demonstrated how to create the SQL string he was interested in creating. I am fully aware of SQL injection attacks and always use typed parameters with stored procedures in "my" solutions. Maybe you should try being less arrogant next time.
|
|
|
|
|
Gerald Schwab wrote: Uhh, he asked what he should concatenate to the end of the SQLString string variable in order to generate a SQL statement that would filter by Parts.ID
No, he didn't ask what he should concatenate on to the end of the string, he said:
string SQLString = "Select * FROM Costs"; //What else to put here??
How do I programatically say, 'strPartNumberInput is Parts.ID'
Gerald Schwab wrote: I simply demonstrated how to create the SQL string he was interested in creating. I am fully aware of SQL injection attacks and always use typed parameters with stored procedures in "my" solutions.
Then why give people answer that lead them down the road to a very well known yet easily correctable security flaw? I doesn't matter one jot if you use parameters in "your" solutions if you don't share the benefits with other people.
Gerald Schwab wrote: Maybe you should try being less arrogant next time.
I'm just trying to do my bit to ensure the world is a more secure place. I don't want my credit card details, or any other of my details, falling into the hands of criminal gangs because someone didn't know how to secure their system properly because someone was lazy in answering a question on a forum.
People have a tendancy to do "just enough" to get something working without really thinking about the security holes they are leaving open in the process. That could be because they don't know about them, or don't care. I can't help with the latter but I can do something about the former.
But, if you think that it is arrogant of me to try and help others secure their systems.....
"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
|
|
|
|
|
As i have understood from your description, you need this SQL query
String SQLString="SELECT * FROM Costs WHERE PartID= "+
"(SELECT ID FROM Parts "+
"WHERE PartNumber="+txtPartNumber.Text+")";
This query is right if your part number is not the part id which stored seprately in parts table along with part id. But if Part no. and part id are the same then you could use following query:
String SQLString="SELECT * FROM Costs WHERE PartID= "+txtPartNumber.Text;
I think it should work....
Wasif Ehsan
|
|
|
|
|
Wasif Ehsan wrote: String SQLString="SELECT * FROM Costs WHERE PartID= "+txtPartNumber.Text;
Your solution is susceptable to a SQL Injection Attack. Please learn how to defend against these attacks as they could compromise your systems. For more information see SQL Injection Attacks and Tips on How to Prevent Them[^]
"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
|
|
|
|
|
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
|
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
