CodeSOD: Injectables are Fun

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.

[Advertisement] Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!

This post originally appeared on The Daily WTF.