please tell me why this query not working

Question

I have in the DB Sid and password and it contain the Sid=senan and the password=pass1234 when I enter this details into my login screen always I get fail messagebox

SqlDataAdapter cmd = new SqlDataAdapter("select Count(*) from [user] where Sid=' " + textBox1.Text + " ' and password='" + textBox2.Text + "'", cnn);
DataTable dt = new DataTable();
cmd.Fill(dt);
if (dt.Rows[0][0].ToString() == "1")
    MessageBox.Show("pass");
else
    MessageBox.Show("fail");

have you tried stepping through your code to see what values are being passed? — jbutler483, Sep 10 '14 at 10:14
On a side-note - you really need to fix that terrible SQL-injection vulnerability. — Cylindric, Sep 10 '14 at 10:15
[Give me parameterized SQL, or give me death](http://blog.codinghorror.com/give-me-parameterized-sql-or-give-me-death/) — Sriram Sakthivel, Sep 10 '14 at 10:17
thank you all it's working, @Cylindric colud you tell me some tips to avoid sql injections — Senan Zedan, Sep 10 '14 at 10:26
@SenanZedan the parameterized approach is the key to avoid Sql Injection — Steve, Sep 10 '14 at 10:26

score 0 · Accepted Answer · edited May 23 '17 at 11:57

The error arises from a simple typo. You have spaces added to the value passed for the Sid condition.
However your query should be rewritten in this way

string cmdText = "select Count(*) from [user] where Sid=@sid and password=@pwd";
SqlCommand cmd = new SqlCommand(cmdText, cnn)
cmd.Parameters.AddWithValue("@sid", textBox1.Text);
cmd.Parameters.AddWithValue("@pwd", textBox2.Text);
int count = Convert.ToInt32(cmd.ExecuteScalar());
if (count > 0)
    MessageBox.Show("pass");
else
    MessageBox.Show("fail");

This approach uses a parameterized query to avoid Sql Injection, and uses directly an SqlCommand without building the SqlDataAdapter and a DataTable. The SqlCommand.ExecuteScalar is the correct method to use when you need to retrieve simply the first column of a single row or call a scalar T-SQL function like COUNT()

As a side note, keep in mind that storing passwords in clear text in your database is a big security concern. Whoever has access to the database will be able to read the passwords of every user. The passwords should be stored as a computed hash and checked repeating the hashing algorithm on the user entered data.

thank you for all this information I will read how to use the hash to protect my passwords. — Senan Zedan, Sep 10 '14 at 10:34

please tell me why this query not working

1 Answers1