Этот код предотвращает внедрение SQL?

Question

Фон

Я заключил контракт на анализ существующего поставщика данных, и я знаю, что следующий код неисправен; но чтобы показать, насколько это плохо, мне нужно доказать, что он подвержен SQL-инъекции.

Вопрос

Какой параметр "Key" может нарушить функцию PrepareString и позволить мне выполнить оператор DROP?

Фрагмент кода

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

Answer 1

В ответ на ваш прямой вопрос: предотвращает ли этот код SQL-инъекцию: Нет

Вот доказательство - протолкните эту строку через метод PrepareString:

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

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

Я изменил опубликованный вами метод GetRecord, чтобы он возвращал полностью подготовленную строку SQL, а не получал запись из базы данных:

Console.WriteLine(GetRecord(output))

И это вывод

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

Добавить 1 дополнительную строку кода:

My.Computer.Clipboard.SetText(input)

И у вас есть строка, которую нужно скопировать прямо в буфер обмена, чтобы вставить в поле ввода на веб-сайте для завершения SQL-инъекции:

'; Drop Table TableName; - -

[отмечая, что управляющие символы были опущены в выводе поста StackOverflow, поэтому вам придется следовать примеру кода для создания вывода]

После запуска метода PrepareString он будет иметь точно такой же результат - код Chr (8) ASCII - это обратная область, которая удалит лишний «», который вы добавляете к моему, который закроет вашу строку, а затем Я свободен, чтобы добавить все, что я хочу в конце. Ваша PrepareString не видит моего - потому что я на самом деле использую - символа возврата, чтобы удалить пробел.

Полученный в результате SQL-код будет беспрепятственно выполнять мой оператор Drop Table и быстро игнорировать остальную часть вашего запроса.

Самое интересное в том, что вы можете использовать непечатаемые символы, чтобы обойти любую проверку символов, которую вы можете придумать. Поэтому безопаснее использовать параметризованные запросы (это не то, что вы просили, но лучший способ избежать этого).

Answer 2

Чтобы ответить на ваш сомнительный вопрос, нет, это не сработает.

.Replace("``", "''") будет препятствовать законным запросам с '`'

.Replace("´", "''") будет препятствовать законным запросам с '´'

.Replace("--", "") будет препятствовать законным запросам с '-' в них

.Replace("''", "'") некорректно изменил бы допустимые запросы с '' '' в них

и т. Д.

Кроме того, полный набор управляющих символов может варьироваться от одной СУБД к другой. Параметризованные запросы FTW.

Answer 3

Я думаю, что это невозможно, если вы просто замените «на». Я слышал, что можно изменить символ escape-кавычки, который потенциально может нарушить это, однако я не уверен. Я думаю, что вы в безопасности, хотя.

Answer 4

Я думаю, что это безопасно (по крайней мере, на сервере SQL), и я также думаю, что единственное, что вам действительно нужно сделать, это s = s.Replace("'", "''"). Конечно, вы должны использовать параметризованные запросы, но вы уже знаете это.

Answer 5

Эта статья MSDN охватывает большую часть того, на что нужно обратить внимание (боюсь сказать все, что касается внедрения SQL).

Но я повторю мнение всех остальных о параметрах.

Что касается вашего примера, некоторые ошибки [Правка: Обновлены эти]:

• не будет ли строка "1 ИЛИ 1 = 1" позволить пользователю вернуть все обратно

• или хуже "1; отбрасывать таблицу sometablename"

Согласно статье, которую вы хотите проверить:

; - Разделитель запросов.

'- символьный разделитель строк данных.

- - Разделитель комментариев.

/ * ... / - Разделители комментариев. Текст между / и * / не оценивается сервер.

xp_ - используется в начале названия расширенные каталогом хранимые процедуры, например, xp_cmdshell.

Answer 6

Если вы попытаетесь использовать свой код, кто-нибудь может передать ключ (; выберите * из таблицы; и получите список таблиц, которые ему нужны.

В вашем коде вы не проверяете точку с запятой, которая позволяет вам завершить оператор t-sql и запустить другой.

Я бы пошел с параметризованным запросом.

Answer 7

Вы пытаетесь создать черный список символов для реализации собственной версии SQL Escaping. Я бы посоветовал просмотреть этот URL - экранирование SQL не обязательно является наихудшим выбором (т. Е. Быстрое исправление существующих приложений), но это необходимо сделать правильно, чтобы избежать уязвимостей.

Этот URL-адрес ссылается на другую страницу для перехода в SQL Server, где автор дает советы, которые помогут вам избежать уязвимостей без ограничения функциональности.

Если это поможет, в статьях также предлагается избегать скобок (я называю их квадратными скобками - но []).