NoName
NoName

Reputation: 1025

How to prevent SQL Injection when executing the script concatenated from parameter strings?

As the same topic, I want to prevent SQL Injection when executing sql command as below:

Dim strSQL As String = "ALTER TABLE " & tablename & " ADD " & fieldName & " " & datatype
_db.Execute_NonQuery(strSQL)

I try applying the solution parameterized http://software-security.sans.org/developer-how-to/fix-sql-injection-microsoft-.net-with-parameterized-queries but I still got this message

The name of the category.

Value: Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection')

Description: A more detailed description of the type of flaw.

Value: This database query contains a SQL injection flaw. The call to system_data_dll.System.Data.IDbCommand.ExecuteNonQuery() constructs a dynamic SQL query using a variable derived from user-supplied input. An attacker could exploit this flaw to execute arbitrary SQL queries against the database. ExecuteNonQuery() was called on an object, which contains tainted data. The tainted data originated from earlier calls to system_data_dll.system.data.sqlclient.sqlcommand.executescalar, and system_data_dll.system.data.common.dbdataadapter.fill. Avoid dynamically constructing SQL queries. Instead, use parameterized prepared statements to prevent the database from interpreting the contents of bind variables as part of the query. Always validate user-supplied input to ensure that it conforms to the expected format, using centralized data validation routines when possible. References: CWE (http://cwe.mitre.org/data/definitions/89.html) OWASP (http://www.owasp.org/index.php/SQL_injection) WASC (http://webappsec.pbworks.com/SQL-Injection)

UPDATE

Original code:

Public Shared Sub AlterTable(ByVal table As String, ByVal fieldName As String, ByVal fieldType As String)

    Dim strSQL As String = "ALTER TABLE " & PROJ & fileCode & " ALTER COLUMN " & fieldName & " " & fieldType
    _db.Execute_NonQuery(strSQL)

End Sub

Public Overloads Function Execute_NonQuery(ByVal sql As String) As Integer
    Dim result As Integer = 0

    Try
        Using conn As New SqlConnection(connString)
            conn.Open()
            If conn IsNot Nothing Then
                Using myTrans As SqlTransaction = conn.BeginTransaction()
                    Using oCmd As SqlCommand = New CommonDao().GetCommand(conn, sql, CommandType.Text)
                        If (oCmd IsNot Nothing) Then
                            oCmd.Transaction = myTrans

                            result = oCmd.ExecuteNonQuery()
                            myTrans.Commit()
                        End If
                    End Using
                End Using
            End If
        End Using
    Catch ex As Exception
        _logger.Error("SQL: " & sql)
        _logger.Error("Error: " & ex.Message)
        Throw ex
    End Try
    Return result
End Function

My modified code

Public Shared Sub AlterTable(ByVal table As String, ByVal fieldName As String, ByVal fieldType As String)

    Dim strSQL As String = "ALTER TABLE @table ALTER COLUMN @fieldName @fieldType"
    _db.Execute_NonQuery(strSQL, New String() {"@table","@fieldName","@fieldType"}, New Object() {table, fieldName, fieldType}, False, CommandType.Text)

End Sub

Public Overloads Function Execute_NonQuery(ByVal spName As String, ByVal param() As String, ByVal values() As Object, ByVal orderNum As Boolean, ByVal commandType As CommandType) As Integer
    Dim result As Integer = 0

    Try
        Using conn As New SqlConnection(connString)
            conn.Open()
            If conn IsNot Nothing Then
                Using oCmd As SqlCommand = New CommonDAO().GetCommand(conn, spName, commandType)
                    If (oCmd IsNot Nothing) Then
                        If Not (param Is Nothing) AndAlso (param.Length > 0) Then
                            For i = 0 To param.Length - 1
                                oCmd.Parameters.Add(New SqlParameter(param(i), values(i)))
                            Next
                        End If

                        result = oCmd.ExecuteNonQuery()
                    End If
                End Using
            End If
        End Using
    Catch ex As Exception
        _logger.Error("SQL: " & spName)
        _logger.Error("Error: " & ex.Message)

        Throw ex
    End Try

    Return result
End Function

Upvotes: 0

Views: 1102

Answers (1)

LoztInSpace
LoztInSpace

Reputation: 5697

Dynamically altering tables in response to user input is quite a bizarre approach. I am almost certain you are doing something incorrectly here.

Do you need to add a column or do you just need to redesign your DB with another table and some joins?

Now having said that, you can probably hide the problem by pushing the dynamic SQL into a stored procedure and executing that instead. It's not nice but if you're just responding to a warning and there's no way this code could be executed from an untrusted source it might be the most pragmatic.

The closest I could find with a quick Google is this:

https://technet.microsoft.com/en-us/library/ms162203(v=sql.90).aspx

which is VB but I'm sure you could work it out.

Upvotes: 2

Related Questions