Preventing SQL from injection attacks

Question

I have a program that contains SQL queries, and it was pointed out to me yesterday that it was wide open to SQL injection attacks. After doing some research, I could see that to fix this, I needed to use parameters instead. I have the following code... How do paramterise this?

Public Shared Function SaveNewPerson(ByVal firstName As String, lastName As String, ByVal age As Integer, ByVal postcode As String, m_cn As OleDbConnection)

    Dim tr As OleDbTransaction = Nothing

    Try
        tr = m_cn.BeginTransaction()

        Dim Dc As New OleDbCommand
        Dc.Connection = m_cn

        Dc.CommandText = "INSERT INTO tblPerson([firstName], [lastName], [age], [postcode]) VALUES('" & firstName & "', '" & lastName & "', '" & age & "', '" & postcode & "')"
        Dc.Transaction = tr
        Dc.ExecuteNonQuery()

        Dim personID As Integer

        Dc.CommandText = "SELECT SCOPE_IDENTITY() AS personID"
        Dc.CommandType = CommandType.Text
        personID = CType(Dc.ExecuteScalar(), Integer)

        tr.Commit()

    Catch ex As Exception

        tr.Rollback()

        Throw
    End Try

End Function

Possible duplicate of [Passing parameter to query for Access database](http://stackoverflow.com/questions/7472563/passing-parameter-to-query-for-access-database) — Matt Wilko, Jul 19 '16 at 10:01
@MattWilko Hi Matt, I'm using SQLServer, rather than an Access database, I probably should have made this clear in the question, but that's why I put that OleDb was a weird choice — , Jul 19 '16 at 10:05
Please post your table schema/definition. also, what db server are you using? — fcm, Jul 19 '16 at 10:27
@fcm there is no definition, I created it in SQLServer before beginning the vb project — , Jul 19 '16 at 10:29
Access uses OLEDB I believe so the solution posted there will work for you — Matt Wilko, Jul 19 '16 at 10:35
@MattWilko Okay I'll try that, thanks, but in that example the SQL gives values to insert (constants), but this wouldn't work for values that are unknown and change each time, surely? — , Jul 19 '16 at 10:40
If you're using Sql Server, you should not be using OleDb. You should be using the classes in `System.Data.SqlClient`. They are optimized specifically for Sql Server. — Chris Dunaway, Jul 20 '16 at 13:27

score 0 · Answer 1 · answered Jul 19 '16 at 10:09

0

I would make a stored procedure first to insert into the sql server and then use

    dc.commandText = "Your stored procedure name"
    dc.commandType = CommandType.StoredProcedure
    Dim myParam as oledb.OleDbParameter = dc.parameters.add("@personID", oledbtype.int)
    myParam.Direction = ParameterDirection.ReturnValue
    dc.Parameters.Add("@firstName", OleDbType.VarChar).Value = [firstname]
    ....
    ....

    Dim returnId as Integer = Cint(dc.Parameters("@personID").Value)

answered Jul 19 '16 at 10:09

Leprechaun

349
2
16

But I need the SQL statement to insert the data into the database, and then after that create an ID for each record, rather than save it and then in a separate function return the ID. I think the answer below is probably better, as it works bar one error that I get. – Jul 19 '16 at 10:27
@joe This will get you the id after it inserts the record. You do this in the stored procedure DECLARE '@Id' AS INT SET '@Id' = SCOPE_IDENTITY() RETURN '@Id.' – Leprechaun Jul 19 '16 at 10:43

Richard · Answer 2 · 2016-07-19T11:26:08.313

-1

   Dc.CommandText = "INSERT INTO tblPerson([firstName], [lastName], [age], [postcode]) VALUES('" & firstName & "', '" & lastName & "', '" & age & "', '" & postcode & "')"

Change this to

Dc.CommandText = "INSERT INTO tblPerson([firstName], [lastName], [age], [postcode]) VALUES(?, ?, ?, ?)"
Dc.Parameters.Add("@first", OleDbType.VarChar, firstName)
Dc.Parameters.Add("@last", OleDbType.VarChar, lastName)
Dc.Parameters.Add("@age", OleDbType.Integer, age)
Dc.Parameters.Add("@postcode", OleDbType.VarChar, postcode )

(Check the right OldDbType value is passed.)

NB. the order in the parameters collection determines which parameter matches which ? placeholder. The names given to the parameters are (it seems) ignored.

edited Jul 19 '16 at 11:26

answered Jul 19 '16 at 09:56

Richard

106,783
21
203
265

1

I believe you have to use the `?` placeholder for OLEDB query parameters? – Matt Wilko Jul 19 '16 at 10:00
@MattWilko Using this code directly copied and pasted from Richards answer. The data I am entering is _firstName_ as **Test**, _lastName_ as **User**, _age_ as **30** and _postcode_ as **ABC 123** I currently get an error on the line `Dc.Parameters.Add("@first", OleDbType.VarChar, firstName)` saying `Conversion from string "Test" to type 'Integer' is not valid.` Is this the reason for it, needing to use question marks? – Jul 19 '16 at 10:04
@joe You may be right, updated to use correct parameter placeholder. – Richard Jul 19 '16 at 11:26
@Richard Hi, sorry this still hasn't worked :( I've created a new question where I have a working statement but with one error at the end, if you're able to help with this one though? http://stackoverflow.com/questions/38457341/sql-transaction-dbnull-error – Jul 19 '16 at 11:36
@Joe: that new question is already answered. Perhaps you need just one question but ensure it is complete code (eg. there is no "test" value in this question to be converted). – Richard Jul 19 '16 at 12:52
@Richard The values are all being passed in from text boxes on the form. During debugging, the parameters are all all being correctly passed in as they return the values that I entered in the relevant textboxes. – Jul 19 '16 at 13:03
@joe You need to do a type conversion to the correct type (the DB can, but it becomes harder to handle errors eg. when a non-number value is submitted for age). – Richard Jul 19 '16 at 13:04
@Richard This isn't possible, each field textbox is validated to only accept certain characters (eg. age can only accept numbers, names only letters and a hyphen etc) – Jul 19 '16 at 13:26

Preventing SQL from injection attacks

2 Answers2