0

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
5
  • 3
    What does the fix you made look like? Commented Feb 29, 2016 at 7:41
  • You shoukd add the code you have now. Commented Feb 29, 2016 at 7:46
  • @guys, I have updated the original and modified code of the problem Commented Feb 29, 2016 at 7:55
  • I am not sure this creates valid SQL, this approach should put single quotes around your input as they are strings, for example: ALTER TABLE 'tablename' ADD 'fieldname' 'VARCHAR(50)' and this is invalid SQL. Commented Feb 29, 2016 at 8:04
  • You're correct, Jaco. I did choose a wrong solution :W Commented Feb 29, 2016 at 8:07

1 Answer 1

2

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.

Sign up to request clarification or add additional context in comments.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.