another VB.net issue

[SteveOOOsw]

I'm trying to add a record into an access table. I'm getting the error 'Syntax error in insert statement.' But, when I try the insert statement in access it works. Have no idea what is going on or how to fix it. Any suggestions would be much appreciated. Here is the code.

Public Shared Sub AddNew(ByVal aAccount As Account, ByVal aEmployee As Employee)
        strUserName = aAccount.GetUserName()
        strPassword = aAccount.GetPassword()
        intEmployeeID = Convert.ToInt32(aEmployee.GetEmployeeID())

        Dim sql As String = "INSERT INTO Account (UserName, Password, EmployeeID) VALUES ('Test', 'Test', 12)"
        '"INSERT INTO Account (UserName, Password, EmployeeID) VALUES ('" & strUserName & "', '" & strPassword & "', " & intEmployeeID & ")"

        Dim adpAccount As New OleDbDataAdapter()
        Try
            adpAccount.InsertCommand = New OleDbCommand(sql)
            adpAccount.InsertCommand.Connection = aConnection
            adpAccount.InsertCommand.ExecuteNonQuery()
        Catch ee As OleDb.OleDbException
            MessageBox.Show(ee.Message.ToString())
        End Try
End Sub

The account table has the following fields...
AccountID - pk AutoNumber
UserName - text
Password - text
EmployeeID - fk number

 

[mikeblas]

Yet another post with injection-vulnerable SQL. It's an epidemic!

What's the statement you execute directly, successfully? Are Password or UserName reserved words in SQL?

 

[SteveOOOsw]

Thanks for the quick reply. Just checked the long list of reserved words and neither were on it. As far as being vulnerable to SQL injection, not too worried about that because this is simply a class project.

Here is an example of a sql statement captured during debugging.

INSERT INTO Account (UserName, Password, EmployeeID) VALUES ('Dwight', 'Secret', 12)

This statement throws an error when I try to execute it with the application but, if I execute it in Access it works.

 

[mikeblas]

As far as being vulnerable to SQL injection, not too worried about that because this is simply a class project.

I don't understand your response. Do your class projects not to be correct? If you're not going to learn about database security in school, when will you learn it?

Meanwhile, I think "Username" and "Password" are both reserved words for OLEDB. (See This list.) Please try enclosing them in square brackets:

   Dim sql As String = "INSERT INTO Account ([UserName], [Password], EmployeeID) VALUES ('Test', 'Test', 12)"

and tell us how you do. When you work directly with Access, you're not using OLEDB.

 

[SteveOOOsw]

Thanks for all your help. Enclosing UserName and Password in brackets did the trick.

Database security is not something we are taught at my university. I've only heard about it only from a guest speaker. He showed us how much information can be attained through SQL injection or how much damage can be caused. I know it is very important in real world applications but for the sake of our class project it is not something that the professor is worried about.

 

[mikeblas]

I'm glad you've got it working, but I'm sad that you're not spending the effort to learn how to do it right.

 

[booyaa]

    Public Function sanitize(ByVal strInput As String) As String
        'replace all ' with escaped ''
        Return Regex.Replace(strInput, "'", "''")
    End Function '---------------------------------------------

just toss in something like this... It's also needed so that if someone types in "it's" your query doesn't go tits up. Anything with a ' in it

Dim sql As String = "INSERT INTO Account (UserName, Password, EmployeeID) VALUES ('Test', 'Test', 12)"
        '"INSERT INTO Account (UserName, Password, EmployeeID) VALUES ('" & sanitize(strUserName) & "', '" & sanitize(strPassword) & "', " & intEmployeeID & ")"

 

[mikeblas]

This use of regex causes more bugs than it fixes.

 

[booyaa]

Return strInput.Replace("'", "''")

the idea is to escape the '
I personally have never seen using regex cause a problem.

 

[mikeblas]

This approach only guards against appostrophies. It doesn't guard against other modes of injection attack, so the server is still wide open.

 

[PurpleFlux]

Just say no to concatenating values in your SQL string.

It really isn't much more difficult to use parameters instead of concatenating your values in the SQL statement string. In fact one might say it is easier since you don't have to mess with trying to filter the input so much.

 

[PTNL]

Just say no to concatenating values in your SQL string.

It really isn't much more difficult to use parameters instead of concatenating your values in the SQL statement string. In fact one might say it is easier since you don't have to mess with trying to filter the input so much.

+1 on that.

It's usually better to force a limited set of possibilities, than to allow everything except whatever you are aware of to filter.