Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Does this code prevent SQL injection?

Background

I've been contracted to analyze an existing Data Provider and I know the following code is faulty; but in order to point out how bad it is, I need to prove that it's susceptible to SQL injection.

Question

What "Key" parameter could break the PrepareString function and allow me to execute a DROP statement?

Code Snippet

Public Shared Function GetRecord(ByVal Key As String) As Record
    Dim Sql As New StringBuilder()

    With Sql
        .Append("SELECT * FROM TableName")
        If String.IsNullOrEmpty(Agency) Then
            .Append(" ORDER BY DateAdded")
        Else
            .Append(" WHERE Key = '")
            .Append(PrepareString(Key))
            .Append("'")
        End If
    End With

    Return ExecuteQuery(Sql.ToString())
End Function

Public Shared Function PrepareString(ByVal Value As String) As String
    Return Value.Replace("''", "'") _
                .Replace("'", "''") _
                .Replace("`", "''") _
                .Replace("´", "''") _
                .Replace("--", "")
End Function
like image 802
Josh Stodola Avatar asked Nov 25 '09 21:11

Josh Stodola


People also ask

Which will prevent SQL injection?

The only sure way to prevent SQL Injection attacks is input validation and parametrized queries including prepared statements. The application code should never use the input directly. The developer must sanitize all input, not only web form inputs such as login forms.

Does HTML encode prevent SQL injection?

The single character that enables SQL injection is the SQL string delimer ' , also known as hex 27 or decimal 39. This character is represented in the same way in SQL and in HTML. So an HTML encode does not affect SQL injection attacks at all.

Do parameters prevent SQL injection?

Use prepared statements and parameterized queries. These are SQL statements that are sent to and parsed by the database server separately from any parameters. This way it is impossible for an attacker to inject malicious SQL.

Can SQL injections be detected?

SQL Injection has become a common issue with database-driven web sites. The flaw is easily detected, and easily exploited, and as such, any site or software package with even a minimal user base is likely to be subject to an attempted attack of this kind.


1 Answers

In answer to your direct question: Does this code prevent SQL injection: No

Here's the proof - push this string through the PrepareString method:

Dim input = "'" & Chr(8) & "; Drop Table TableName; - " & Chr(8) & "-"
Dim output = PrepareString(input)

Console.WriteLine(input)
Console.WriteLine(output)

I modified the GetRecord method you posted to return the fully prepared SQL string rather than get the record from the database:

Console.WriteLine(GetRecord(output))

And this is the output

Input  = ; Drop Table TableName; --
Output = '; Drop Table TableName; --
Query  = SELECT * FROM TableName WHERE Key = ''; Drop Table TableName; --'

Add 1 extra line of code:

My.Computer.Clipboard.SetText(input)

And you've got the string you need copied right to your clipboard to paste into your input field on the website to complete your SQL injection:

'; Drop Table TableName; - -

[Noting that the control characters have been omitted from the post output by StackOverflow, so you'll have to follow the code example to create your output]

After the PrepareString method is run, it will have the exact same output - the Chr(8) ASCII code is the backspace which will remove the extra "'" that you're appending to mine which will close your string and then I'm free to add whatever I want on the end. Your PrepareString doesn't see my -- because I'm actually using - - with a backspace character to remove the space.

The resulting SQL code that you're building will then execute my Drop Table statement unhindered and promptly ignore the rest of your query.

The fun thing about this is that you can use non-printable characters to basically bypass any character check you can invent. So it's safest to use parameterized queries (which isn't what you asked, but is the best path to avoid this).

like image 125
BenAlabaster Avatar answered Sep 30 '22 02:09

BenAlabaster