String problem with SQL Reader?

Question

Function FillAdminAccount() As Boolean

    FillAdminAccount = True

           Try

        SQLconn.ConnectionString = "connect timeout=9999999;" & _
                                   "data source=" & DefaultIserver & ";" & _
                                   "initial catalog=" & DefaultIdBase & "; " & _
                                   "user id=userid;" & _
                                   "password=userpass;" & _
                                   "persist security info=True; " & _
                                   "packet size=4096"
        SQLconn.Open()

        SQLcmd.CommandType = CommandType.Text

        SQLcmd.CommandText = "Select distinct username, cast(convert(varchar,userpassword) as varchar) as 'userpassword' from " & tblUsersList & " where usertype='MainAdmin'"
        SQLcmd.Connection = SQLconn

        SQLreader = SQLcmd.ExecuteReader

        While SQLreader.Read = True
            CurrentAdminUser = SQLreader("username").ToString
            CurrentAdminPass = SQLreader("userpassword").ToString   'PROBLEM'
        End While

    Catch ex As Exception
        ErrorMessage(ex)
    Finally
        If SQLconn.State = ConnectionState.Open Then SQLconn.Close()
        If SQLreader.IsClosed = False Then SQLreader.Close()
    End Try

End Function    'FillAdminAccount

Please see the line with the comment PROBLEM. On this code, the output is equal to "userpassword. As you can see, there is no quotation mark on the right and I wonder why. By the way, the data type of the userpassword in the database is BINARY. Wish you could help me on this. Thank you..x_x

Answer 1

Could it be

as varchar) as 'userpassword'

should be

...as varchar) as [userpassword] ..

or

...as varchar) as userpassword ..

Answer 2

NEVER store actual passwords in the db. Now it looks like your passwords might not quite be plain text because of the convert/cast operations, but you still have a problem. At very least any encryption used is easily reversible, and if your sql server ever ends up on a different machine from the application then passwords will be traveling over the wire in plain text.

If you MUST do this (perhaps because of a legacy system or mandate from above) then at least do the matching at the server so that the password never comes back to the application.

What you should be doing is using something like SQL Server 2005's HashBytes() function to only store a hash of the actual password. When someone tries to login, hash their attempted password and match up the hashes.

As to your specific question, my guess is the cast or convert operation failed resulting in a NULL value coming back to the application. And do you have both a CAST() and a CONVERT() to the same type? It's redundant.

Answer 3

@Oglester is right, it's the single quotes around 'userpassword'.

This is not a bug, but it's just dumb:

 cast(convert(varchar,userpassword) as varchar

You can use cast or convert but there's no point in using both.