System.Data.SqlClient.SqlException: 'Incorrect syntax near 'USERNAME'.'

Question

I'm trying to make a login and I keep getting this error:

System.Data.SqlClient.SqlException: 'Incorrect syntax near 'USERNAME'.'

Here is my code

SqlConnection con = new SqlConnection(@"Data Source=(LocalDB)\MSSQLLocalDB;AttachDbFilename=C:\Users\Omar\Documents\Data.mdf;Integrated Security=True;Connect Timeout=30");
SqlDataAdapter sda = new SqlDataAdapter("Select Count(*)  From [LOGIN] were USERNAME ='" + textBox1.Text +"' and PASSWORD='"+ textBox2.Text +"'",con);
DataTable dt = new DataTable();

sda.Fill(dt);

Answer 1

You're getting voted down not just because this is a duplicate question but because your attempt wraps two security problems in a single piece of code.

So let's take this step by step.

1) Your actual problem is you mispelt WHERE. Your corrected code would look something like

SqlConnection con = new SqlConnection(@"Data Source=(LocalDB)\MSSQLLocalDB;AttachDbFilename=C:\Users\Omar\Documents\Data.mdf;Integrated Security=True;Connect Timeout=30");
SqlDataAdapter sda = new SqlDataAdapter("Select Count(*)  From [LOGIN] where USERNAME ='" + textBox1.Text +"' and PASSWORD='"+ textBox2.Text +"'",con);
DataTable dt = new DataTable();
sda.Fill(dt);

But you don't need a data adapter or a table, you can return a count from SQL directly. So you could do something like

string sql = "select count(*) from users where username = '" + username + "' and password = '" + password + "'";
using (SqlConnection conn = new SqlConnection(connStr))
{
    conn.Open();

    using (SqlCommand command = new SqlCommand(query, conn))
    {
        int result = (int)command.ExecuteScalar();
        if (result > 0)
        {
            /// login sucessful
        }
    }
}

This would work, but you have a security vulnerability called SQL injection.

If we look at a correct login your SQL string would be

select count(*) from login where username = 'alice' and password = 'bob'

This works fine, but if I enter the classic example of SQL injection for a login page as the password, ' OR 1=1 -- then your SQL string becomes this;

select count(*) from login where username = 'alice' and password = '' OR 1=1 --'

This line of SQL will always return 1, because it's highjacked the SQL via SQL injection, adding an OR clause at the end of the statement, OR 1=1 which is always going to be true. It then uses SQL comment syntax to comment out whatever comes after it. So now I can login as anyone at all, even usernames that don't exist.

2) The correct way to build SQL strings, if you don't want to use ORMs that do this for you (and really, use an ORM, all the protection is automatic) is to use a parameterized query, which takes the input and formats it in such a way that any special characters aren't taken as SQL commands, like so

string sql = "select count(*) from login where username = @username and password = @password";
using (SqlConnection conn = new SqlConnection(connStr))
{
    conn.Open();

    using (SqlCommand command = new SqlCommand(query, conn))
    {
        command.Parameters.Add(new SqlParameter("@username", username));
        command.Parameters.Add(new SqlParameter("@password", password));
        int result = (int)command.ExecuteScalar();
        if (result > 0)
        {
            /// login sucessful
        }
    }
}

The parameters are in the SQL query as @parameterName, and then added with command.Parameters.Add(). Now you've avoid SQL injection

3) However this still a security problem. You're storing your passwords in plaintext. If I can gain access to your SQL database (via SQL injection, or you leaving the server open to the world) once I have a copy of the database I have your usernames and passwords and your company is going to end up on haveibeenpwned. You shouldn't be doing this. Passwords should be protected, not via encryption, but via what is called hashing and salting. This transforms the passwords into a value that is derived from it via a one way function, it takes the password, feeds it into some math, and the result out the other side represents the password, but you can't go back the other way, you can't figure out the password from the salted hash. Then when you compare logins, you compute the hash again, and use that in your comparison, in an ORM driven query or a parameterised query.

Now, with all that in mind, I'd strongly advise you to avoid doing any of this yourself. C# & ASP.NET have libraries for usernames and passwords in ASP.NET Identity. I would much rather see you use that, not just because I happen to be the PM owner for that and for .NET Security as a whole, but because it does everything right for you, without you having to do any work.

If this is for a real world application please take some time to go through the OWASP project, it has lots of examples of security problems and details on how to fix them.

Answer 2

Please...

1. learn about parameters; the code you have is open to SQL injection, which is a huge problem (especially, but not just, on a login form)
2. learn about not storing plain-text passwords, instead using salted cryptographic hashes
3. learn about IDisposable; multiple of the objects involved here are disposable and need using
4. don't use DataTable unless you know why you're using it - there is no reason whatsoever to use it here
5. learn about tools that will help do all of the above for you

and when, and only when, you have the above "down": fix the typo

Here's a version using "dapper", that covers most of this:

using (var con = SqlConnection(@"...not shown..."))
{
    var hash = HashPasswordUsingUsernameAsSalt(
        textBox1.Text, textBox2.Text); // impl not shown
    int count = con.QuerySingle<int>(@"
        select COUNT(1) from [LOGIN]
        where [USERNAME]=@cn and [PW_HASH]=@hash",
        new { cn = textBox1.Text, hash });
    // TODO: do something with count
}

Answer 3

Look like on your 2nd line you have a typo for the WHERE clause.

SqlDataAdapter sda = new SqlDataAdapter("Select Count(*)  From [LOGIN] WHERE USERNAME ='" + textBox1.Text +"' and PASSWORD='"+ textBox2.Text +"'",con);

However, you need to learn about SQL injection attacks since your SQL is wide open to hack attempts.