Verify credentials from database

Question

I had confusion with my code:

Dim sqladapter As SqlDataAdapter = New SqlDataAdapter()
Dim sqlcmd As SqlCommand = New SqlCommand()
sqlcmd = New SqlCommand("SELECT login, pass from Table1 where login=" & login.Text & "and pass='" & password.Text.ToString() & "';", connect)
Dim dr As SqlDataReader = sqlcmd.ExecuteReader()
Dim dt As DataTable = New DataTable()
dt.Load(dr)
If (dt.Rows.Count = 1) Then
'Display welcome page or do some action here.

Now, my question is, is there any other way of doing Rows.Count==1 . I'm feeling that it is very wrong and makes no sense at.

How do you verify from database that a user has only one valid record in table other than counting rows.

Thanks in Advance :)

(Please ask me before reporting question)

http://stackoverflow.com/questions/332365/how-does-the-sql-injection-from-the-bobby-tables-xkcd-comic-work — Ňɏssa Pøngjǣrdenlarp, Sep 14 '14 at 21:21
Your code is vulnerable to injection, you should be using [parameterised queries](http://stackoverflow.com/questions/542510/how-do-i-create-a-parameterized-sql-query-why-should-i). — The Blue Dog, Sep 14 '14 at 21:21
I think injection isn't your only problem if you're storing plaintext passwords... — Dan Puzey, Sep 14 '14 at 21:23
Fix the injection, fix the plain text passwords, then use SqlCommand.ExecuteScalar with IF EXISTS — Steve, Sep 14 '14 at 21:26

score 2 · Accepted Answer · edited May 23 '17 at 12:29

You have two problems, one is called Sql Injection and you have already numerous links that explain why is really bad. Another one is the plain text password stored in your database. This is a big security concern because everyone that has the possibility to look at your database could see the passwords of your users. (The gravity of this, of course, is linked to the nature of your application but cannot be downplayed) See this link for an answer on how to hash a string (a password) and get its encrypted version to store in the database instead of the plain text.

Finally the code you use could be changed to avoid both the SqlDataAdapter and the DataTable.
Just use an ExecuteScalar against an IF EXIST query that return just 1 if the user/password exists or zero if not

Dim cmdText = "IF EXISTS(SELECT 1 FROM Table1 WHERE login = @log AND pass = @pwd) " & _
              "SELECT 1 ELSE SELECT 0"
using connect = new SqlConnection(connectionstring)
using sqlcmd = New SqlCommand(cmdText, connect)
    connect.Open()
    sqlcmd.Parameters.AddWithValue("@log", login.Text)
    sqlcmd.Parameters.AddWithValue("@pwd", password.Text) ' <- Subst with a call to an hash function 
    Dim exists = Convert.ToInt32(sqlcmd.ExecuteScalar())
    if exists = 1 Then
         'Display welcome page or do some action 
    else

    end if
End Using
End Using

Thanks @steve. I really appreciate your help. God Bless ya :) — Sikander Hayyat, Sep 19 '14 at 21:10

score 0 · Answer 2 · answered Sep 14 '14 at 21:17

There is only one way to answer to the question and its to count rows. The different solution would be to count them in database. For example you could write stored procedure that takes username and password and returns boolean this way you would drag less data.

As a side note there is potential sql injection in your code. You should not store clear password in database. You should return the whole row and match hash of the password from database to the hash of the paasword that you get from UI.

Verify credentials from database

2 Answers2