Today, Morpheus sends us a SQL injection vulnerability. But it's a peculiar version that only uses parameters. Let's start with the bit that looks normal:
strStrBuilder.Append(" update sometable set ")
strStrBuilder.Append(" SOMECOLUMN = :p_somevalue, ")
strStrBuilder.Append(" rowuserid = :p_userid, ")
strStrBuilder.Append(" rowtaskid = :p_taskid ")
strStrBuilder.Append(" where id = :p_id")
strSQL = strStrBuilder.ToString
This is VB.Net code, and while I'm never a huge fan of building SQL queries by appending strings together, this is fine. It's the rest of the context that makes it horrible:
Public Function SomeMethod(ByVal int1 As Integer, ByVal int2 As Integer, Optional ByVal strSQL As String = "") As Boolean
Dim intRecordsAffected As Integer = 0
Dim bResult As Boolean = False
Dim strStrBuilder As New StringBuilder()
If bUseLocalConnect Then
m_cnn.Open()
m_tx = m_cnn.BeginTransaction()
End If
If strSQL.Equals(String.Empty) Then
strStrBuilder.Append(" update sometable set ")
strStrBuilder.Append(" SOMECOLUMN = :p_somevalue, ")
strStrBuilder.Append(" rowuserid = :p_userid, ")
strStrBuilder.Append(" rowtaskid = :p_taskid ")
strStrBuilder.Append(" where id = :p_id")
strSQL = strStrBuilder.ToString
End If
Dim cmd As New SqlClient.SqlCommand(strSQL, m_cnn)
cmd.Transaction = m_tx
' Snip: use the command
Return True
End Function
This method accepts an optional strSQL
parameter. If that parameter is supplied… we just execute that query. Whatever it is. Hopefully it came from somewhere without any user inputs, but who knows? I don't! It's no big deal, just a method that runs arbitrary SQL against the database with no protections or validations.
I know why these kinds of methods get built: someone wanted a backdoor, not for any nefarious purpose, but just because sometimes doing things the "right" way takes too long, or requires too many other people. Sometimes, you just want to say, "Oh, I know what's wrong, and I can fix it with this little query." It's a seductive whisper, but always a bad idea.
Morpheus writes: "Lucky for us it isn't actually called anywhere within the application."
It isn't called anywhere within the application right now. But I bet it has been, and I bet the developer responsible wants to use it again.