Invalid Column name while column is there

Question

I am trying to create a login window with MDI. It is connected to a SQL Server table test. I have changed the datatypes and deleted and recreated the DB. I have 2 columns: usr and pwd of datatype nvarchar.

Dim connetionString As String
Dim cnn As SqlConnection

connetionString = "Data Source=.;Initial Catalog=test;User ID=sa;Password=sasql"
cnn = New SqlConnection(connetionString)

Dim cmd As SqlCommand
Dim myreader As SqlDataReader
Dim query As String

query = "Select usr From users WHERE (usr =" + TextBox1.Text + " and pwd = " + TextBox2.Text + ")"
cmd = New SqlCommand(query, cnn)

cnn.Open()
myreader = cmd.ExecuteReader()

If myreader.Read() Then
Else
    MessageBox.Show("Incorrect username/password !", "LOGIN ERROR", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
End If

cnn.Close()

Thank you so much.

To fix your specific problem you need to add quotes in your SQL string around the values coming in from the db. But that's the least of your worries. Look up parameterization and SQL injection. Stop and do that NOW, before going any further. — sasfrog, Apr 17 '16 at 08:55
Thx sasfrog. I'm a beginner and I do it for fun. I will go deep into it. — Jad, Apr 17 '16 at 09:04
The best approach for when you're building a string is to inspect it once you've built it. You can see without quotes why you got that message from the db. — sasfrog, Apr 17 '16 at 09:08

score 1 · Accepted Answer · answered Apr 17 '16 at 09:44

If you are using Tortuga.Chain, the code would look like this:

Dim ds As New SqlServerDataSource(connetionString)

Dim user = ds.From("users", new With {.usr = TextBox1.Text, .pwd = TextBox2.Text}).ToString.Execute();

If user Is Not Nothing Then
Else
    MessageBox.Show("Incorrect username/password !", "LOGIN ERROR", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
End If

If you want to stick to raw ADO.NET code, you need to use a parameterized query.

Dim connetionString As String
Dim cnn As SqlConnection

connetionString = "Data Source=.;Initial Catalog=test;User ID=sa;Password=sasql"
cnn = New SqlConnection(connetionString)

Dim cmd As SqlCommand
Dim myreader As SqlDataReader
Dim query As String

query = "Select usr From users WHERE (usr = @user and pwd = @pwd )"
cmd = New SqlCommand(query, cnn)
cmd.Parameters.AddWithValue( "@usr", TextBox1.Text)
cmd.Parameters.AddWithValue( "@pwd", TextBox2.Text)

cnn.Open()
myreader = cmd.ExecuteReader()

If myreader.Read() Then
Else
    MessageBox.Show("Incorrect username/password !", "LOGIN ERROR", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
End If

cnn.Close()

score 0 · Answer 2 · edited May 23 '17 at 10:27

0

Remove the spaces from the value of connectionString.

Your query compares textual data, which is invalid unless the user enters ' at the start and end of the textboxes. This would be syntactically correct:

"Select usr From users WHERE (usr ='" + TextBox1.Text + "' and pwd = '" + TextBox2.Text + "')"

but logically flawed, since the user might attempt to inject malicious SQL into your textboxes. Your application is extremely unsafe. You must protect it against SQL injection and you need to encrypt the password of the user as well.

edited May 23 '17 at 10:27

Community

1
1

answered Apr 17 '16 at 09:08

Lajos Arpad

64,414
37
100
175

You know it's wrong, so why offer an example that doesn't use a parameterized query? – Jonathan Allen Apr 17 '16 at 09:44
@JonathanAllen, who "offered" that example? Which part of "but logically flawed" confused you? I described that it would be sntactically correct, but logically flawed. So you disagree with my statement by agreeing to it. Congratulations :) – Lajos Arpad Apr 17 '16 at 09:50
It isn't just "logically flawed", it flat out doesn't work. If my username is "O'Malley" I'll break your code even without attempting a SQL injection attack. – Jonathan Allen Apr 17 '16 at 09:58
@JonathanAllen, there are cases when it will not fail. Which means that your statement "it flat out doesn't work" is false. You are pointing out that there is a case when it will fail, which is true. That's what we, developers call bugs. Which are logical flaws. This article will introduce you into the concept of "bug": https://en.wikipedia.org/wiki/Software_bug – Lajos Arpad Apr 17 '16 at 10:04
@JonathanAllen, as about your statement that parameterized query must be used, I must tell you, that is flawed as well. If he wants to enter the parameters into the query but escapes them properly or guarantees otherwise that there will be no problems (mentioned in my answer as well). For instance, if foo is a number, then I see no problem with "select name from buyers where id=" + foo. I specified that injection attacks should be prevented and if they are prevented well, then the error mentioned by you will not occur. So, if you will, your statement that parameterized queries are needed is – Lajos Arpad Apr 17 '16 at 10:09
misleading beginners. It is certainly a semi-standard way, but claiming that they will be needed is nonsense. – Lajos Arpad Apr 17 '16 at 10:09
"I'll break your code even without attempting a SQL injection attack." It is not my code. It is the closest to the op's code that works, which illustrates that his lack of apostrophe led him to the syntactical error. I have never said that the code is acceptable. I explained why is it flawed, but it seems your selective attention filtered out that. – Lajos Arpad Apr 17 '16 at 10:14

Invalid Column name while column is there

2 Answers2