error in query in asp.net

Question

Error is showing that invalid column name mustufain. mustufain is the value of UserName.Text.toString()

string query = "select userid from register where username = " + UserName.Text.ToString() + " and " + "password = " + Password.Text.ToString();

SqlCommand cmd1 = new SqlCommand(query,connection);
connection.Open();
SqlDataReader rd1 = cmd1.ExecuteReader();
while(rd1.Read())
{
    Session["checkuserid"] = rd1["userid"];
}
connection.Close();

First, ASP.NET doesn't execute queries. That's ADO.NET. Second, you should *not* construct sql statements by concatenation. Use parameterized queries. In this case you have no idea what the user typed. What if it was `1;drop table users;--` ? — Panagiotis Kanavos, Apr 15 '16 at 14:50
SqlConnection, SqlCommand and SqlDataReader implement [IDisposable](https://msdn.microsoft.com/en-us/library/system.idisposable(v=vs.110).aspx). You need to either call dispose in a `finally` block on them, or wrap them in a [using statement](https://msdn.microsoft.com/en-us/library/yh598w02.aspx) to ensure they get disposed of properly. If you don't do this now, you're going to run into issues later and it will be hard to track the issues down. — mason, Apr 15 '16 at 14:51
And it appears you are storing plaintext passwords in your database. Don't do that! That's a security issue. Passwords should be one-way hashed and salted to make it difficult for an attack to retrieve the plaintext password. To test to see if a user has entered the correct password, you compared the hashed copies. — mason, Apr 15 '16 at 14:54

Rion Williams · Answer 1 · 2016-04-15T14:55:53.987

Firstly, you should not be using string concatenation to build your queries as it can leave you vulnerable to things like SQL Injection attacks and it can cause issues with your queries being incorrect (as you are missing tick marks around your parameters) :

// This would attempt to state username = mustufain instead of 
// username = 'mustufain' (and SQL doesn't know what mustufain is)
var query = "select userid from register where username = '" + UserName.Text + "' and " + "password = '" + Password.Text + "'";

A better approach using parameterization would look like the following, which avoids the incorrect syntax and offers you protection against any nasty injections :

// Open your connection
using(var connection = new SqlConnection("{your connection string}"))
{
     // Build your query
     var query = "SELECT TOP 1 userid FROM register WHERE username = @username AND password = @password";
     // Build a command (to execute your query)
     using(var command = new SqlCommand(query, connection))
     {
          // Open your connection
          connection.Open();
          // Add your parameters
          command.Parameters.AddWithValue("@username",UserName.Text);
          command.Parameters.AddWithValue("@password",Password.Text);
          // Execute your query
          var user = Convert.ToString(command.ExecuteScalar());
          // If a user was found, then set it
          if(!String.IsNullOrEmpty(user))
          {
               Session["checkuserid"] = user;
          }
          else
          {
               // No user was found, consider alerting the user
          }
     }
}

Finally, you may want to reconsider how you are storing your credentials (in clear text). ASP.NET offers a wide variety of providers that can help handle this process for you so that you don't have to do it yourself.

score 3 · Answer 2 · edited May 23 '17 at 12:07

You are trying to concatenate strings to build an sql query and, as usual, you get errors. In your specific case you forget to enclose your string values between single quotes. But the only correct way to do this query is by the way of a parameterized query

string query = @"select userid from register 
                where username = @name and password = @pwd";
using(SqlCommand cmd1 = new SqlCommand(query,connection))
{
    connection.Open();
    cmd1.Parameters.Add("@name", SqlDbType.NVarChar).Value = UserName.Text;
    cmd1.Parameters.Add("@pwd", SqlDbType.NVarChar).Value = Password.Text;
    using(SqlDataReader rd1 = cmd1.ExecuteReader())
    {
        ....
    }
}

Notice also that storing passwords in clear text in your database is a very bad practice and a strong security risk. On this site there are numerous questions and answers that explain how to create an hash of your password and store that hash instead of the clear text

For example: Best way to store passwords in a database

error in query in asp.net

2 Answers2