[RESOLVED] Preventing SQL injection

First off I'm using SQL to read and all that held within an access database. I wish to prevent SQL injection since how I"m doing it and what I'm reading that I don't completely understand I am a possible victim to attack at some point if I don't fix it. here are snippets of code relevant to my login form.

http://pastebin.com/jCGBTVWK

Can you paste the code, here, using the code tags?

I can it's just a lot of code.

[[[modUniversal.vb]]]

Code:

---


Module modUniversal
    Public state As String = "0"
 
    Structure accountInformation
        Dim ID As Integer
        Dim username As String
        Dim permissions As Integer
        Dim firstName As String
        Dim middleName As String
        Dim lastName As String
    End Structure
    Public user As accountInformation
End Module

---

[[[modLogin.vb]]]

Code:

---


Module modLogin
 
    Sub validation(credential As String)
        Select Case state
            Case "0"
                state = "0.1"
                loginDB("username, password", "username", login.usernameTextbox.Text)
            Case "0.1"
                state = "0.2"
                loginDB("userID, username, permissions, firstName, middleName, lastName", "username", user.username)
        End Select
    End Sub
End Module

---

[[[login.vb]]]

Code:

---


Public Class login
    Private Sub login_Load(sender As Object, e As EventArgs) Handles MyBase.Load
        If My.Settings.rememberedUsername IsNot "" Then
            usernameTextbox.Text = My.Settings.rememberedUsername
            rememberMe_checkbox.Checked = True
            passwordTextbox.Select()
        End If
    End Sub
 
    Private Sub exitButton_Click(sender As Object, e As EventArgs) Handles exitButton.Click
        Environment.Exit(0)
    End Sub
 
    Private Sub loginButton_Click(sender As Object, e As EventArgs) Handles loginButton.Click
        Select Case Len(usernameTextbox.Text) >= 7 AndAlso Len(passwordTextbox.Text) >= 10
            Case True
                modDatabase_login.loginDB("username, active", "username", usernameTextbox.Text)
            Case False
                If Len(usernameTextbox.Text) < 7 AndAlso Len(passwordTextbox.Text) < 10 Then
                    MessageBox.Show("Invalid username and password length")
                ElseIf Len(usernameTextbox.Text) < 7 Then
                    MessageBox.Show("Invalid username length")
                ElseIf Len(passwordTextbox.Text) < 10 Then
                    MessageBox.Show("Invalid password length")
                End If
        End Select
    End Sub
 
    Private Sub rememberMe_checkbox_Click(sender As Object, e As EventArgs) Handles rememberMe_checkbox.Click
        If rememberMe_checkbox.Checked = True Then
            My.Settings.rememberedUsername = usernameTextbox.Text
        Else
            My.Settings.rememberedUsername = ""
        End If
    End Sub
 
    Private Sub usernameTextbox_TextChanged(sender As Object, e As EventArgs) Handles usernameTextbox.TextChanged
        If rememberMe_checkbox.Checked = True Then
            rememberMe_checkbox.Checked = False
        End If
    End Sub
End Class

---

[[[modDatabase_login.vb]]]

Code:

---


Imports System.Data.OleDb
 
Module modDatabase_login
    ' Make parameter for connection status and applicationEvents can have a test for that connection status but needs a fail-safe for networks without access to the internet.
  Sub loginDB(columns As String, comparisonColumn As String, comparisonString As String)
        Dim command As OleDbCommand
        ' This connection will be created over the network (will have priority of local network if at site to being outside the network completely) when finished and locally for temporary offline use
      Dim connection = New OleDbConnection("Provider=Microsoft.ACE.OLEDB.12.0;Data Source=C:\Users\Shawn\Desktop\HPCR.accdb")
        Dim SQL As String = "SELECT " & columns & " FROM accounts WHERE " & comparisonColumn & " = """ & comparisonString & """"
        command = New OleDbCommand(SQL, connection)
        Connection.Open()
        Dim reader As OleDbDataReader = command.ExecuteReader()
        If reader.Read() Then
            Select Case state
                Case "0"
                    If Convert.ToBoolean(reader("active")) Then
                        user.username = Convert.ToString(reader(comparisonColumn))
                        validation(comparisonString)
                    Else
                        MessageBox.Show("Account not active")
                    End If
                Case "0.1"
                    If Convert.ToString(reader("password")) = login.passwordTextbox.Text Then
                        validation(comparisonString)
                    Else
                        state = "0"
                        MessageBox.Show("Invalid password")
                    End If
                Case "0.2"
                    user.ID = Convert.ToInt32(reader("userID"))
                    user.permissions = Convert.ToInt32(reader("permissions"))
                    user.firstName = Convert.ToString(reader("firstName"))
                    user.middleName = Convert.ToString(reader("middleName"))
                    user.lastName = Convert.ToString(reader("lastName"))
                    login.Hide()
                    mainMenu.Show()
                    login.Close()
                    state = "1"
            End Select
        Else
            state = "0"
            MessageBox.Show("Invalid username")
        End If
        connection.Dispose()
        connection.Close()
        login.passwordTextbox.Clear()
        If state = "0" Then
            login.passwordTextbox.Focus()
        End If
    End Sub
 
End Module

---

SQL injection is a serious issue and cannot always be stopped.

Your first step is to use parameterized queries, I suggest reading up on JMcIlhinney's blog: Using Parameters in ADO.NET.

Your second step is to validate the length of values, for example if the value expected is a DateTime and the value inputted is 25 characters long then you can expect that the value entered is not a valid value. Another way to validate values is to try to convert them to their data type before inputting them. For example, if the expected value is numeric then use TryParse to see if the value entered can be converted to a number. Another way to validate values is to use controls that return the target data type(ie - NumericUpDown for numbers, DateTimePicker for dates, etc.).

Your third step would be to never allow escape sequences or comment characters such as: ;, ', --, and /* ... */

Of course these are very basic steps, but they are very important to handle.

change to.

Code:

---

        command = New OleDbCommand(SQL, connection)
        Connection.Open()

        Dim SQL As String = "SELECT " & columns & " FROM accounts WHERE @compariColumn = @compariString"

        command.Parameters.AddWithValue("@compariColumn, comparisonColumn)
        command.Parameters.AddWithValue("@compariString, comparisonString)

---

Should work

Should work.

Quote:

---

Originally Posted by dday9

Your third step would be to never allow escape sequences or comment characters such as: ;, ', --, and /* ... */

---

I agree with the majority of what you have said, however, users should be allowed to enter weird characters. Like ' or ; or --, for example, in usernames, what if I wanted my username to be 'Toph?.... I'm too tired to type right now

Well, the apostrophe is one that should be allowed to use in certain situations such as O'Malley but the double hyphen should never be allowed.

There are times when a double hyphen is valid data, so it should be allowed.

Thankfully using parameters means that escape sequences etc can safely be used, as the parameters deal with the issues themselves.

Quote:

---

SQL injection is a serious issue and cannot always be stopped.

---

Really? not that its a serious issue but why don't you think it can be stopped?

Quote:

---

why don't you think it can be stopped?

---

The "stock" way of preventing it is to use parameters as DD linked to in post 4, however there are some situations where a parameter cannot be used, e.g. to substitute a table or column name. I'm not sure I'd agree that injection attacks cannot be stopped in these scenarios but it becomes a lot more difficult.

My personal approach is to ban special characters in these scenarios and I design my systems with that in mid. If I absolutely cannot ban these characters due to some bizarre user requirement, the best I've ever been able to come up with is to wrap the input in delimiters to force the content to be treated as a whole unit and then parse the input escaping any delimiters it contained. That should, in theory, prevent an injection but I've never been 100% confident of that and I've never been able to find a rock solid confirmation that it couldn't be broken. All I can say with certainty is that I can't find a way to break it and nobody has managed to do so on any of the systems I've rolled out yet.


edit> It probably goes without saying that if anyone can think of a way to break it I'd rather they didn't post it here but I'd love to get a pm.

Quote:

---

Originally Posted by dday9

Your first step is to use parameterized queries, I suggest reading up on JMcIlhinney's blog: Using Parameters in ADO.NET.

Your second step is to validate the length of values, for example if the value expected is a DateTime and the value inputted is 25 characters long then you can expect that the value entered is not a valid value. Another way to validate values is to try to convert them to their data type before inputting them. For example, if the expected value is numeric then use TryParse to see if the value entered can be converted to a number. Another way to validate values is to use controls that return the target data type(ie - NumericUpDown for numbers, DateTimePicker for dates, etc.).

Your third step would be to never allow escape sequences or comment characters such as: ;, ', --, and /* ... */

---

The first and second steps should be combined: convert to the appropriate (VB) type in your VB code (or use controls that return that type as you mention) and pass those converted values as query parameters.

The third step is unnecessary and (as noted) causes problems with valid input.

Another layer of defence should be to connect to the database with the minimum permissions required (definitely not the MySQL equivalent of sa)

I'm not sure how to read a boolean value or anything from the database all I find it values that are given from something in the program because there all insert into and I want select from where

Quote:

---

Originally Posted by Toph

change to.

Code:

---

        command = New OleDbCommand(SQL, connection)
        Connection.Open()

        Dim SQL As String = "SELECT " & columns & " FROM accounts WHERE @compariColumn = @compariString"

        command.Parameters.AddWithValue("@compariColumn, comparisonColumn)
        command.Parameters.AddWithValue("@compariString, comparisonString)

---

Should work

Should work.

---

Sorry I totally missed this and just realized how that works but then I got one other question. How should I go about taking the data it finds from the database am I safe continueing the use convert.tostring or whatever datatype or is there a better safer way to do this. I believe I read somewhere tryparse is better so that it checks the datatypes are the same where my datatypes I'm using are same or should be much as it's possible. Thats the last of what I need thank you.

Quote:

---

Originally Posted by fatalerror0x00

Sorry I totally missed this and just realized how that works but then I got one other question. How should I go about taking the data it finds from the database am I safe continueing the use convert.tostring or whatever datatype or is there a better safer way to do this. I believe I read somewhere tryparse is better so that it checks the datatypes are the same where my datatypes I'm using are same or should be much as it's possible. Thats the last of what I need thank you.

---

Retrieve it the way you retrieve it. TryParse() is a separate thing. Learn TryParse() seperatly

Alright so I've attempted to use parameters but it's not working at all.

When I run this little simple test code

Code:

---

Sub test()
        Dim command As OleDbCommand
        Dim connection = New OleDbConnection("Provider=Microsoft.ACE.OLEDB.12.0;Data Source=C:\Users\Shawn\Desktop\HPCR.accdb")
        Dim SQL As String = "SELECT username FROM accounts WHERE @1 = @2"
        command = New OleDbCommand(SQL, connection)
        command.Parameters.AddWithValue("@1", "username")
        command.Parameters.AddWithValue("@2", login.usernameTextbox.Text)
        connection.Open()
        Dim reader As OleDbDataReader = command.ExecuteReader()
        If reader.Read() Then
            user.username = Convert.ToString(reader("username"))
        End If
        MessageBox.Show(user.username)
    End Sub

---

I get the error:

Code:

---

An unhandled exception of type 'System.TypeInitializationException' occurred in managementSystem.exe

Additional information: The type initializer for 'managementSystem.modDatabase_login' threw an exception.

---

Because of the change I just made reverting back to my older code with some changes I'm gonna repaste my codes because I'm still getting that error. It seems like it's something else. The error for my previous post is as soon as the sub is called. Same in my current one soon as I call the very first loginDB sub it errors

[[[login.vb]]]

Code:

---

Imports System.Data.OleDb

Public Class login
    Private Sub login_Load(sender As Object, e As EventArgs) Handles MyBase.Load
        If My.Settings.rememberedUsername IsNot "" Then
            usernameTextbox.Text = My.Settings.rememberedUsername
            rememberMe_checkbox.Checked = True
            passwordTextbox.Select()
        End If
    End Sub

    Private Sub exitButton_Click(sender As Object, e As EventArgs) Handles exitButton.Click
        Environment.Exit(0)
    End Sub

    Private Sub loginButton_Click(sender As Object, e As EventArgs) Handles loginButton.Click
        Select Case Len(usernameTextbox.Text) >= 7 AndAlso Len(passwordTextbox.Text) >= 10
            Case True
                'Dim command As OleDbCommand
                ' This connection will be created over the network (will have priority of local network if at site to being outside the network completely) when finished and locally for temporary offline use
                Dim connection As OleDbConnection = New OleDbConnection("Provider=Microsoft.ACE.OLEDB.12.0;Data Source=C:\Users\Shawn\Desktop\HPCR.accdb")
                modDatabase_login.loginDB("username, active", "username", usernameTextbox.Text)
                Select Case state
                    Case "0.2"
                        modDatabase_login.loginDB("username, password", "username", usernameTextbox.Text)
                    Case "0.3"
                        modDatabase_login.loginDB("userID, username, permissions, firstName, middleName, lastName", "username", user.username)
                End Select
                connection.Close()
                passwordTextbox.Clear()
                If state = "0" Then
                    passwordTextbox.Focus()
                End If
            Case False
                If Len(usernameTextbox.Text) < 7 AndAlso Len(passwordTextbox.Text) < 10 Then
                    MessageBox.Show("Invalid username and password length")
                ElseIf Len(usernameTextbox.Text) < 7 Then
                    MessageBox.Show("Invalid username length")
                ElseIf Len(passwordTextbox.Text) < 10 Then
                    MessageBox.Show("Invalid password length")
                End If
        End Select
    End Sub

    Private Sub rememberMe_checkbox_Click(sender As Object, e As EventArgs) Handles rememberMe_checkbox.Click
        If rememberMe_checkbox.Checked = True Then
            My.Settings.rememberedUsername = usernameTextbox.Text
        Else
            My.Settings.rememberedUsername = ""
        End If
    End Sub

    Private Sub usernameTextbox_TextChanged(sender As Object, e As EventArgs) Handles usernameTextbox.TextChanged
        If rememberMe_checkbox.Checked = True Then
            rememberMe_checkbox.Checked = False
        End If
    End Sub
End Class

---

[[[modDatabase_login.vb]]]

Code:

---

Imports System.Data.OleDb

Module modDatabase_login
    Dim command As OleDbCommand
    ' This connection will be created over the network (will have priority of local network if at site to being outside the network completely) when finished and locally for temporary offline use
    Dim connection As OleDbConnection = New OleDbConnection("Provider=Microsoft.ACE.OLEDB.12.0;Data Source=C:\Users\Shawn\Desktop\HPCR.accdb")
    Dim reader As OleDbDataReader = command.ExecuteReader()
    ' Make parameter for connection status and applicationEvents can have a test for that connection status but needs a fail-safe for networks without access to the internet.
    Sub loginDB(columns As String, comparisonColumn As String, comparisonString As String)
        Dim SQL As String = "SELECT " & columns & " FROM accounts WHERE " & comparisonColumn & " = """ & comparisonString & """"
        command = New OleDbCommand(SQL, connection)
        If connection.State = ConnectionState.Closed Then
            connection.Open()
        End If
        If reader.Read() Then
            Select Case state
                Case "0.1"
                    If Convert.ToBoolean(reader("active")) Then
                        user.username = Convert.ToString(reader(comparisonColumn))
                        state = "0.2"
                    Else
                        state = "0"
                        MessageBox.Show("Account not active")
                    End If
                Case "0.2"
                    If Convert.ToString(reader("password")) = login.passwordTextbox.Text Then
                        state = "0.3"
                    Else
                        state = "0"
                        MessageBox.Show("Invalid password")
                    End If
                Case "0.3"
                    user.ID = Convert.ToInt32(reader("userID"))
                    user.permissions = Convert.ToInt32(reader("permissions"))
                    user.firstName = Convert.ToString(reader("firstName"))
                    user.middleName = Convert.ToString(reader("middleName"))
                    user.lastName = Convert.ToString(reader("lastName"))
                    login.Hide()
                    mainMenu.Show()
                    login.Close()
                    state = "1"
            End Select
        Else
            state = "0"
            MessageBox.Show("Invalid username")
        End If
    End Sub

End Module

---

[[[modUniversal.vb]]]

Code:

---

Module modUniversal
    Public state As String = "0"

    Structure accountInformation
        Dim ID As Integer
        Dim username As String
        Dim permissions As Integer
        Dim firstName As String
        Dim middleName As String
        Dim lastName As String
    End Structure
    Public user As accountInformation
End Module

---

Currently there is no modLogin.vb module file anymore just cause I got rid of it cause for this version I really don't need it

You should only be using parameters for values, not field names.

Try changing these lines to this:

Code:

---

Dim SQL As String = "SELECT username FROM accounts WHERE username = @username"
        command = New OleDbCommand(SQL, connection)
        command.Parameters.AddWithValue("@username", login.usernameTextbox.Text)
        connection.Open()

---

That still fails just the same as before but thank you for letting me know that information that helps

I just noticed this gives two errors which the error that I wasn't seeing at first seems to probably be the underlying cause of my second error.

A first chance exception of type 'System.NullReferenceException' occurred in managementSystem.exe
A first chance exception of type 'System.TypeInitializationException' occurred in managementSystem.exe

In mod_databaseLogin you're initialising a field when you initialise the type (Dim reader As OleDbDataReader = command.ExecuteReader()) with an expression that relies on command field that isn't initialised until you run the loginDB method.

I think you meant to have that expression in the loginDB method itself. (In fact I think you could move the field declaration and make it a local variable as well)

Alright so now I've got this error when I fixed what you were saying

An unhandled exception of type 'System.NullReferenceException' occurred in managementSystem.exe

Additional information: Object reference not set to an instance of an object.

immediate window this is displayed: A first chance exception of type 'System.NullReferenceException' occurred in managementSystem.exe

oh wait it helps if I give you some code. I'm pointing to this sub before I do any other login stuff I had previously and I get the previous posts error message

Code:

---

Sub test()
    Dim command As OleDbCommand
    Dim connection As OleDbConnection = New OleDbConnection("Provider=Microsoft.ACE.OLEDB.12.0;Data Source=C:\Users\Shawn\Desktop\HPCR.accdb")
        Dim SQL As String = "SELECT username FROM accounts WHERE username = @username"
        command.Parameters.AddWithValue("@username", login.usernameTextbox.Text)
        command = New OleDbCommand(SQL, connection)
        If connection.State = ConnectionState.Closed Then
            connection.Open()
        End If
        Dim reader As OleDbDataReader = command.ExecuteReader()
        If reader.Read() Then
            user.username = Convert.ToString(reader("username"))
            MessageBox.Show(user.username)
        End If
    End Sub

---

wait I fixed it XD yay! I promise after I have my login process switched to my new login i'll mark this as solved