Solved Password Validation sub

[mikenyby]

Hello all!

I'm hoping someone can check my work to see if there are any red flags with how I've handled this.

I am working on an Access interface linked to a SQL Server Backend, and just completed a password validation routine.

To start, I have a trigger in the SQL Server table that converts inserted passwords into salted and hashed strings. Paired with that I have a SQL Server Function called "passvalid" that salts and hashes the password entered in the login attempt and checks it against the string saved in the users table, returning a bit. I'm using a pass-through query to call the function from Access and check the return. I've copied the sub below. Thanks!

Private Sub cmdLogIn_Click()

    Dim intID As Integer
    Dim strPassword As String
    
    intID = Me.cboUser
    strPassword = Me.txtPassword
    
    With CurrentDb.QueryDefs("qryPassValid")
        .SQL = "SELECT dbo.passvalid(" & intID & ", '" & strPassword & "') AS valid"
        .ReturnsRecords = True
    End With
    
    Dim Valid As Integer
        Valid = DLookup("valid", "qryPassValid")
    
    
    Select Case Valid
    
        Case 0
            MsgBox "Invalid password.", vbOKOnly, "Login"
            Me.txtPassword = ""
            Me.txtPassword.SetFocus
            
        Case Else
            'Routine that opens program
        End Select
End Sub

 

[theDBguy]

Not sure which red flags you're looking for; but if it works, then I guess it's okay. I would think, though, that using Windows Authentication might be more secured than using a password. Are you not able to use that?

 

[mikenyby]

Certainly Windows authentication is secure and I have that running as well. The issue is this program will be installed on two separate workstations that multiple users will be accessing. They won't be switching out the Windows user, only logging into the program. Plus, some users will have access to more features and I'm not sure if there's a way to set that using Windows authentication.

What I'm not sure about is if there are any security red flags such as SQL injection that might work here (pretty sure it won't given the passwords are stored only in the salted & hashed format), or if the code itself is working in a roundabout way that could be cleaned up.

 

[Pat Hartman]

On a computer used by multiple users, a log in form is safer.

 

[mikenyby]

Alright, well the good news is the login form seems entirely functional. Thanks you both. I'll won't mark this solved just yet in case anyone else shows up with some insight.

 

[cheekybuddha]

Hmmm...

What happens if you use the following password in Me.txtPassword:

') AS ignore, '1'='1

 

[mikenyby]

Hmmm...

What happens if you use the following password in Me.txtPassword:

') AS ignore, '1'='1

We get an ODBC error:

The debug highlights
Valid = DLookup("valid", "qryPassValid")

I suppose I should put some error handling in here to prevent users from screwing around in the VBA.

 

[The_Doc_Man]

This kind of error that Dave described is called an "SQL Injection" attack. It is common enough to have its own name, so protecting against it is a good idea.

 

[mikenyby]

This kind of error that Dave described is called an "SQL Injection" attack. It is common enough to have its own name, so protecting against it is a good idea.

Fortunately it doesn't seem to work! I thought that using a hash procedure and function would probably do the trick, but I haven't had to program something like this in the past.

 

[cheekybuddha]

We get an ODBC error:

OK, try this variation (I made a slight mistake in the last one):

') AS ignore, ('1'='1

or:

'),('1

(NOTE: I have edited these - test again if you have used the original tests!)

 

[mikenyby]

OK, try this variation (I made a slight mistake in the last one):

') AS ignore, ('1'='1"

or:

'),('1"

The first string brings up the same ODBC error, the second gives us Runtime Error 13, Type Mismatch.

 

[cheekybuddha]

The first string brings up the same ODBC error, the second gives us Runtime Error 13, Type Mismatch.

Sorry - I just edited.

Please try once more with the revised versions above

 

[mikenyby]

Sorry - I just edited.

Please try once more with the revised versions above

This one worked:

'),('1

Do you know a way to protect against this?

 

[cheekybuddha]

Do you know a way to protect against this?

Yes! It's not difficult.

but I have to go and do some stuff now, and won't be able to get back to this for a couple of hours.

Welcome to your first SQL injection

 

[mikenyby]

Yes! It's not difficult.

but I have to go and do some stuff now, and won't be able to get back to this for a couple of hours.

Welcome to your first SQL injection

I put this in and it protects against that specific attack, but it won't protect against any other syntax that might work:

    If strPassword = "'),('1" Then
        MsgBox "No.", vbOKOnly, "No."
        Me.txtPassword = ""
        Me.txtPassword.SetFocus
        Exit Sub
    End If

 

[cheekybuddha]

I put this in and it protects against that specific attack, but it won't protect against any other syntax that might work

Do you understand how the attack works?

Add a Debug.Print .SQL before End With and evaluate the query you have created

As you say, your If block will not protect you against variations.

 

[mikenyby]

Do you understand how the attack works?

Add a Debug.Print .SQL before End With and evaluate the query you have created

As you say, your If block will not protect you against variations.

I do understand how it works, it alters the query that the password validation routine is based off of, here forcing a return of '1'. What I slapped together is a simple stop gap. I frankly don't know what variations would also work. SQL injection is something I've only read about but don't have any hands on experienced dealing with.

Looking at the SQL queries in the immediate window, it seems that disallowing any string that contains single or double quotes or a space would protect from this style of injection attack, but I'm going to do some deep googling to get a better understanding of injection defense.

 

[cheekybuddha]

Looking at the SQL queries in the immediate window, it seems that disallowing any string that contains single or double quotes or a space would protect from this style of injection attack

But then you prevent users using those characters in their password.

My advice is to create a stored procedure on the server that accepts the Integer ID and the password. Use those parameters directly to return your query result.

Then use parameters in your VBA pass-through to call the stored proc.

On my phone so can't offer examples till later

 

[mikenyby]

Thanks @cheekybuddha

I'll work on it and look forward to your examples.

 

[cheekybuddha]

OK, the following is just aircode, so more of a guide ...

First create a stored proc on your server:

IF object_id('validate_pw') > 0
  DROP PROCEDURE validate_pw;
GO
 
CREATE PROCEDURE validate_pw
  @id         INT,
  @pw         NVARCHAR(300)  -- adjust size to max accepted by passvalid() function.
AS
BEGIN

  SELECT dbo.passvalid(@id, @pw) AS valid;

END

Then, in your VBA standard module, create a function:

Function PasswordValid(userID As Long, pw As String) As Integer

  Const SP As String = "validate_pw p1, p2;"
  Dim ret  As Integer
 
  With CurrentDb.QueryDefs("qryPasswordValid")
    .SQL = SP
    .ReturnsRecords = True
    .Parameters("p1") = userID
    .Parameters("p2") = pw
    With .OpenRecordset
      ret = .Fields("valid")
      .Close
    End With
  End With
 
  PasswordValid = ret

End Function

Then adjust your button code:

Private Sub cmdLogIn_Click()

    Dim intID As Integer
    Dim strPassword As String
    Dim Valid As Integer
   
    intID = Me.cboUser
    strPassword = Me.txtPassword  
    Valid = PasswordValid(intID, strPassword)  
   
    Select Case Valid
      Case 0
        MsgBox "Invalid password.", vbOKOnly, "Login"
        Me.txtPassword = ""
        Me.txtPassword.SetFocus
      Case Else
        'Routine that opens program
    End Select
   
End Sub