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 BooleanDim intRecordsAffected As Integer = 0Dim bResult As Boolean = FalseDim strStrBuilder As New StringBuilder()If bUseLocalConnect Then m_cnn.Open() m_tx = m_cnn.BeginTransaction()End IfIf 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.ToStringEnd IfDim cmd As New SqlClient.SqlCommand(strSQL, m_cnn)cmd.Transaction = m_tx' Snip: use the commandReturn TrueEnd 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!