Как избежать SQL-инъекций без параметров - PullRequest
107 голосов
/ 26 мая 2009

У нас здесь еще одно обсуждение использования параметризованных SQL-запросов в нашем коде. У нас есть две стороны в обсуждении: я и некоторые другие, которые говорят, что мы всегда должны использовать параметры для защиты от SQL-инъекций, и другие парни, которые не считают это необходимым. Вместо этого они хотят заменить одиночные апострофы двумя апострофами во всех строках, чтобы избежать SQL-инъекций. Все наши базы данных работают на Sql Server 2005 или 2008, а наша база кода работает на .NET Framework 2.0.

Позвольте мне привести простой пример на C #:

Я хочу, чтобы мы использовали это:

string sql = "SELECT * FROM Users WHERE Name=@name";
SqlCommand getUser = new SqlCommand(sql, connection);
getUser.Parameters.AddWithValue("@name", userName);
//... blabla - do something here, this is safe

Пока другие парни хотят это сделать:

string sql = "SELECT * FROM Users WHERE Name=" + SafeDBString(name);
SqlCommand getUser = new SqlCommand(sql, connection);
//... blabla - are we safe now?

Где функция SafeDBString определяется следующим образом:

string SafeDBString(string inputValue) 
{
    return "'" + inputValue.Replace("'", "''") + "'";
}

Теперь, пока мы используем SafeDBString для всех строковых значений в наших запросах, мы должны быть в безопасности. Правильно?

Есть две причины использовать функцию SafeDBString. Во-первых, это так, как это было сделано с давних пор, а во-вторых, легче отлаживать операторы SQL, поскольку вы видите точный запрос, который выполняется в базе данных.

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

Есть ли кто-нибудь, кто может сломать это? Как бы вы это сделали?

EDIT: Подведем итоги ответов:

  • Никто еще не нашел способа обойти SafeDBString на Sql Server 2005 или 2008. Это хорошо, я думаю?
  • В нескольких ответах указывалось, что вы получаете прирост производительности при использовании параметризованных запросов. Причина в том, что планы запросов можно использовать повторно.
  • Мы также согласны с тем, что использование параметризованных запросов дает более читаемый код, который легче поддерживать
  • Кроме того, всегда проще использовать параметры, чем использовать различные версии SafeDBString, преобразования строки в число и преобразования строки в дату.
  • Используя параметры, вы получаете автоматическое преобразование типов, что особенно полезно при работе с датами или десятичными числами.
  • И, наконец: Не пытайтесь самостоятельно обеспечивать безопасность , как писал JulianR. Поставщики баз данных тратят много времени и денег на безопасность. Мы никак не можем добиться большего успеха, и нет причин, по которым мы должны стараться делать их работу.

Так что, хотя никто не смог нарушить простую защиту функции SafeDBString, у меня появилось много других хороших аргументов. Спасибо!

Ответы [ 21 ]

82 голосов
/ 26 мая 2009

Я думаю, что правильный ответ:

Не пытайтесь обеспечивать безопасность самостоятельно . Используйте любую надежную отраслевую стандартную библиотеку, которая вам нужна, вместо того, чтобы пытаться сделать это самостоятельно. Какие бы предположения вы ни делали в отношении безопасности, они могут быть неверными. Каким бы безопасным ни казался ваш собственный подход (и в лучшем случае он выглядит шатким), есть риск, что вы что-то упускаете из виду и действительно ли вы хотите воспользоваться этим шансом, когда речь заходит о безопасности?

Используйте параметры.

71 голосов
/ 26 мая 2009

А потом кто-то идет и использует «вместо». Параметры, IMO, единственный безопасный путь.

Это также позволяет избежать многих проблем с датами и числами в i18n; какая дата 01/02/03? Сколько стоит 123 456? Ваши серверы (app-server и db-server) согласуются друг с другом?

Если фактор риска для них не убедителен, как насчет производительности? СУБД может повторно использовать план запроса, если вы используете параметры, помогающие повысить производительность. Он не может сделать это только с помощью строки.

27 голосов
/ 26 мая 2009

Аргумент беспроигрышный. Если вам удастся найти уязвимость, ваши коллеги просто изменят функцию SafeDBString, чтобы учесть ее, а затем попросят вас доказать, что она снова небезопасна.

Учитывая, что параметризованные запросы бесспорной лучшие практики программирования, бремя доказывания должно быть на них, чтобы указать, почему они не используют метод, который одновременно является более безопасным и более исполнительским.

Если проблема заключается в переписывании всего унаследованного кода, простым компромиссом будет использование параметризованных запросов во всем новом коде и рефакторинг старого кода для их использования при работе с этим кодом.

Полагаю, реальная проблема заключается в гордости и упрямстве, и с этим ничего не поделаешь.

19 голосов
/ 26 мая 2009

Прежде всего, ваш образец для версии «Заменить» неверен. Вы должны поставить апострофы вокруг текста:

string sql = "SELECT * FROM Users WHERE Name='" + SafeDBString(name) & "'";
SqlCommand getUser = new SqlCommand(sql, connection);

Так что это еще одна вещь, которую параметры делают для вас: вам не нужно беспокоиться о том, должно ли значение заключаться в кавычки. Конечно, вы можете встроить это в функцию, но тогда вам нужно добавить к ней много сложности: как узнать разницу между «NULL» как нулем и «NULL» как строкой или между числом и строка, которая просто содержит много цифр. Это просто еще один источник ошибок.

Другая вещь - это производительность: параметризованные планы запросов часто кэшируются лучше, чем составные планы, что, возможно, экономит серверу шаг при выполнении запроса.

Кроме того, экранирование одинарных кавычек недостаточно. Многие продукты БД допускают альтернативные методы экранирования символов, которыми может воспользоваться злоумышленник. В MySQL, например, вы также можете избежать одиночной кавычки с обратной косой чертой. И поэтому следующее значение «name» взорвало бы MySQL только с помощью функции SafeDBString(), потому что когда вы удваиваете одинарную кавычку, первая по-прежнему экранируется обратной косой чертой, а 2-я остается «активной»:

x \ OR 1 = 1; -


Кроме того, JulianR поднимает хороший момент ниже: НИКОГДА попытайтесь сделать работу безопасности самостоятельно. Тонкие способы ошибочного программирования безопасности настолько просты, что кажется работающим, даже при тщательном тестировании. Затем проходит время, и через год вы узнаете, что ваша система была взломана шесть месяцев назад, и вы даже не знали об этом до тех пор.

Всегда полагайтесь как можно больше на библиотеки безопасности, предоставляемые для вашей платформы. Они будут написаны людьми, которые зарабатывают код безопасности для жизни, гораздо лучше протестированы, чем то, чем вы можете управлять, и обслуживаются поставщиком, если обнаружена уязвимость.

10 голосов
/ 26 мая 2009

Так что я бы сказал:

1) Почему вы пытаетесь повторно внедрить что-то встроенное? он доступен, легок в использовании и уже отлажен в глобальном масштабе. Если в нем будут обнаружены будущие ошибки, они будут исправлены и доступны всем очень быстро, без необходимости что-либо предпринимать.

2) Какие процессы существуют для гарантии того, что вы никогда не пропустите вызов SafeDBString? Пропуск всего в одном месте может открыть целый ряд проблем. Сколько вы собираетесь посмотреть на эти вещи, и подумайте, сколько будет потрачено впустую этих усилий, когда принятого правильного ответа так легко достичь.

3) Насколько вы уверены, что вы закрыли каждый вектор атаки, о котором Microsoft (автор БД и библиотеки доступа) знает в вашей реализации SafeDBString ...

4) Насколько легко читать структуру sql? В примере используется + конкатенация, параметры очень похожи на string.Format, который более читабелен.

Кроме того, есть 2 способа выяснить, что на самом деле было выполнено: накатить свою собственную функцию LogCommand, простую функцию с без проблем с безопасностью или даже посмотреть трассировку sql, чтобы определить, какая база данных думает, что действительно происходит.

Наша функция LogCommand просто:

    string LogCommand(SqlCommand cmd)
    {
        StringBuilder sb = new StringBuilder();
        sb.AppendLine(cmd.CommandText);
        foreach (SqlParameter param in cmd.Parameters)
        {
            sb.Append(param.ToString());
            sb.Append(" = \"");
            sb.Append(param.Value.ToString());
            sb.AppendLine("\"");
        }
        return sb.ToString();
    }

Правильно или нет, он дает нам необходимую информацию без проблем с безопасностью.

7 голосов
/ 26 мая 2009

При параметризованных запросах вы получаете больше, чем защита от внедрения SQL. Вы также получаете лучший потенциал кэширования плана выполнения. Если вы используете профилировщик запросов к серверу sql, вы все равно можете увидеть «точный sql, который выполняется в базе данных», так что вы на самом деле ничего не теряете в плане отладки ваших операторов sql.

5 голосов
/ 26 мая 2009

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

4 голосов
/ 26 мая 2009

Вы не можете легко выполнить какую-либо проверку типа пользовательского ввода без использования параметров.

Если вы используете классы SQLCommand и SQLParameter для выполнения вызовов БД, вы все равно можете видеть выполняемый SQL-запрос. Посмотрите на свойство CommandText SQLCommand.

Я всегда подозреваю, что подход «сам по себе» предотвращает внедрение SQL, когда параметризованные запросы так просты в использовании. Во-вторых, просто потому, что «это всегда так делали», не значит, что это правильно.

3 голосов
/ 26 мая 2009

Это безопасно, только если вы гарантированно передадите строку.

Что если вы не передадите строку в какой-то момент? Что если вы передадите просто число?

http://www.mywebsite.com/profile/?id=7;DROP DATABASE DB

В конечном итоге станет:

SELECT * FROM DB WHERE Id = 7;DROP DATABASE DB
2 голосов
/ 26 мая 2009

Огромное согласие по вопросам безопасности.
Другая причина использования параметров - это эффективность.

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

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

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...