Мы продолжаем развивать PVS-Studio как SAST решение. Одно из направлений, в котором ведутся работы, – улучшение покрытия OWASP. А какой же OWASP без taint-анализа? Вот и мы так подумали и решили прикрутить его к C# анализатору. О том, что получилось, а что не очень, ниже.
Примечание. Данная статья не ставит перед собой цель во всех подробностях раскрыть тематику SQL инъекций и работы с SQL из C#. Эти вопросы затрагиваются поверхностно и служат цели дополнения рассказа про taint-анализ.
Упрощённо говоря, с помощью taint-анализа отслеживается распространение по программе данных, полученных из внешнего источника (taint source). Само по себе распространение "заражённых" данных опасности не несёт. Однако есть ключевые точки (taint sinks), при попадании в которые возможно возникновение различных уязвимостей: SQLI, XSS и т.п. Суть taint-анализа заключается в том, чтобы находить такие трассы распространения данных, по которым они могут без валидации попасть из taint source в taint sink.
Для реализации нам нужно будет учесть:
Смешать, но не взбалтывать. ;)
Простенький пример:
var userName = Request.QueryString["userName"];
var rawCommand = "SELECT * FROM users WHERE name = '" + userName + "'";
var sqlCommand = new SqlCommand(rawCommand);
// Execution of the tainted commandВ прямом смысле слова "классический" пример SQL инъекции – если будете гуглить, сразу наткнётесь на что-нибудь подобное. Проблема в том, что данные приходят от пользователя и сразу используются для формирования команды. Как следствие, злоумышленник волен подстроить данные таким образом, чтобы изменить логику SQL-запроса. Ожидаете на вход строку вида JohnSmith? Получите вместо этого входные данные вида Sergey'; DROP TABLE users; --. Распишитесь.
Вообще первым анализатором в PVS-Studio, в котором была реализована концепция taint-анализа, был анализатор C и C++ кода. На taint-анализ опирается диагностическое правило V1010. Оно ищет различные кейсы, базирующиеся на одинаковом принципе: в программу извне поступают данные и без должной проверки приходят туда, куда не должны. Например, в функцию вызова командного интерпретатора – system. С помощью этого правила удалось найти несколько интересных кейсов, которые я описывал в отдельной статье (в ней даже пара видео есть). Ещё на C++ Russia 2018 я делал небольшой доклад, в том числе и на эту тему.
Примечание. Из упомянутой статьи особенно примечательным мне кажется случай, когда в коде исправляли CVE, но что-то пошло не так. В итоге код поправили, проблема никуда не ушла, и через какое-то время ей был присвоен новый CVE-идентификатор. Уже после этого код исправили как нужно. :)
Вообще желание реализовать taint-анализ в C# анализаторе было у нас давно. Наверное, примерно с того самого времени, как он был имплементирован в C++ анализаторе. Мысли "сделать аналог V1010 в C# анализаторе" периодически озвучивались, однако руки всё как-то не доходили. Но, как я упоминал выше, одно из намеченных направлений развития C# анализатора в 2021 году – улучшение покрытия OWASP (в первую очередь – OWASP Top 10 2017, но и про ASVS забывать не хочется). Это стало отличным поводом для того, чтобы руки наконец дошли.
Первой диагностикой, которая будет использовать taint-анализ, решили сделать диагностику по поиску возможных SQL инъекций. Популярная тема, есть упоминание и в OWASP Top 10 2017 (A1:2017-Injection) и в OWASP ASVS – отличный кандидат. На том и порешили.
На базе data-flow анализа. В принципе, основная инфраструктура уже была готова. Что нужно было сделать, так это добавить информацию про источники, приёмники, передачу заражения и сброс заражённого статуса.
Параллельно мы дорабатывали анализатор в тех местах, которые находили: улучшили поддержку интерполированных строк в data-flow, обработку счётчиков циклов, убрали ряд false positives за счёт доработок общих механизмов. В общем, параллельно подкручивали анализатор то тут, то там.
Но вернёмся к taint-анализу.
Вся цепочка распространения заражённых данных по коду приложения начинается с источников. Мы считаем, что источники возвращают заражённые данные безусловно. В общем и целом, в качестве источников выступают те точки приложения, где данные на вход могут поступать от пользователя.
Примеры источников:
Например, в следующем коде мы считаем переменную taintedVariable заражённой.
void Example()
{
var taintedVariable = Console.ReadLine();
TaintSink(taintedVariable);
}В качестве ещё одного типа источников мы приняли параметры методов, доступные внешнему коду. Например, параметры public методов public классов:
public class Helper
{
public void ExecuteCommand(String commandToExecute)
{
TaintSink(commandToExecute);
}
}В данном случае в методе ExecuteCommand мы считаем параметр commandToExecute заражённым. Суть такого подхода в следующем. Если метод доступен для внешней сборки, скорее всего, это API для взаимодействия с библиотекой. Альтернативный вариант – кто-то просто не заморачивается по поводу модификаторов области видимости. :)
Разработчик, использующий библиотеку, может рассчитывать, что данные проверяются внутри вызываемого метода из библиотеки. Даже если код библиотеки открыт и лежит где-нибудь на GitHub, скорее всего, пользователь библиотеки не проверяет имплементацию каждого используемого метода. В общем, пользователь библиотеки может рассчитывать на то, что данные будут проверены внутри вызываемого метода.
Разработчик же самой библиотеки может рассчитывать на то, что на вход методу приходят уже проверенные данные. Значит, в повторной проверке необходимости нет – пришедшие данные можно использовать напрямую.
Таким образом, может возникнуть ситуация, когда данные, пришедшие от пользователя в приложении, не были проверены ни в самом приложении, ни в используемой им библиотеке. Как результат — прямое использование внешних данных, что может привести к возникновению уязвимости.
PVS-Studio не сможет понять, что стоит за вызовом библиотечного метода, если его исходный код недоступен, а сам метод непроаннотирован. Однако ситуации такие обнаруживать всё же хочется. Поэтому, как я писал выше, можно предупреждать самих разработчиков библиотеки о том, что данные из параметра публично доступного метода могут попасть в taint sink без обработки.
Рассмотрим какой-нибудь пример, иллюстрирующий сказанное.
public class DBHelper
{
public void ProcessUserInfo(String userName)
{
....
var command = "SELECT * FROM Users WHERE userName = '" + userName + "'";
ExecuteCommand(command);
....
}
private void ExecuteCommand(String rawCommand)
{
using (SqlConnection connection = new SqlConnection(_connectionString))
{
....
using (var sqlCommand = new SqlCommand(rawCommand, connection))
{
using (var reader = sqlCommand.ExecuteReader())
....
}
}
}
}Метод ProcessUserInfo доступен для использования внешнему коду (public метод в public классе). Параметр userName используется для формирования SQL запроса в строковом представлении (записывается в переменную command). Затем command в качестве аргумента передаётся в метод ExecuteCommand. Соответствующий параметр — rawCommand — используется для создания SQL команды (sqlCommand), которая затем исполняется. Выглядит небезопасно, но явного taint-источника здесь нет.
Предположим, что рассмотренный выше код — часть какой-то библиотеки. Назовём её SQLILib.dll. Эта библиотека используется в приложении (пусть будет SQLIApp.exe), где метод ProcessUserInfo используются следующим образом:
static void TestHelper(DBHelper helper)
{
var userName = Request.Form["userName"];
helper.ProcessUserInfo(userName);
}Здесь данные, полученные от пользователя (Request.Form["userName"]), напрямую передаются в метод ProcessUserInfo. Так как ProcessUserInfo — метод из внешней библиотеки, так просто его код не посмотреть.
В итоге выходит, что данные были получены напрямую от пользователя и переданы в метод, где они также без проверки используются — выглядит опасно.
Да, можно предположить, что между пользователями библиотеки и её разработчиками есть некая договорённость о том, что в методы должны приходить уже проверенные данные. Например, это указано в документации к API. Но, когда речь идёт про безопасность, лучше перестраховаться.
К сожалению, при анализе кода SQLIApp.exe анализатор не сможет предупредить о возможной SQL инъекции, так как ничего не знает про метод ProcessUserInfo. С другой стороны, анализатор может выдать предупреждение при анализе кода самой библиотеки.
Выдавать ли предупреждения, когда источником заражённых данных считаются параметры — решать каждой диагностике, использующей taint-информацию. Для диагностики по поиску SQL инъекций мы решили выдавать предупреждения на низком уровне достоверности.
Примечание. Если вы не хотите видеть подобных предупреждений, то можете отключить их с помощью комментария вида //-V::5608:3 в .pvsconfig файле. Тогда предупреждения V5608 (SQLI) низкого уровня достоверности не попадут в лог. Более подробно про .pvsconfig файлы можно почитать в документации "Подавление ложных предупреждений" (раздел "Подавление ложных предупреждений с помощью файлов конфигурации диагностик (.pvsconfig)").
И наоборот, если вы считаете эти предупреждения крайне важными, то можете повысить их уровень достоверности до выского, используя //V_LEVEL_1::5608. Подробности приводятся в главе "Как задать свой уровень для конкретной диагностики" в разделе документации "Дополнительная настройка диагностик".
Приёмники заражённых данных являются уникальными для каждого диагностического правила. Поэтому они скорее относятся к диагностикам, чем к общему механизму taint-анализа. Как мы обсуждали, суть приёмников в том, что в них не должны попадать заражённые данные. Если в приложении есть путь, когда данные из taint source попадают в taint sink — беда.
Например, в случае с SQL инъекциями, приёмником может служить конструктор класса SQLCommand или метод FromSqlRaw.
Пример:
var taintedStr = GetTaintedData();
var sqlCommand = new SqlCommand(taintedStr); // taint sink
....Можно было бы подумать, что конструктор класса SqlCommand — это, скорее, передатчик, а приёмником выступает какой-нибудь из методов исполнения команды: SqlCommand.ExecuteSomehow. Однако кажется странным сначала создать заражённую команду, а потом пытаться её проверять. Проще и более очевидно проверить данные, на основе которых команда создаётся. Именно поэтому в нашем случае конструктор SqlCommand — sink, а не передатчик данных.
Таким же приёмником является свойство SqlCommand.CommandText. Пример опасного кода:
void ProcessUserInfo()
{
using (SqlConnection connection = new SqlConnection(_connectionString))
{
....
String userName = Request.Form["userName"];
using (var command = new SqlCommand()
{
Connection = connection,
CommandText = "SELECT * FROM Users WHERE UserName = '" + userName + "'",
CommandType = System.Data.CommandType.Text
})
{
using (var reader = command.ExecuteReader())
....
}
}
}Здесь создаётся экземпляр типа SqlCommand, однако заражённая строка не передаётся в качестве аргумента в конструктор, а используется для инициализации свойства CommandText.
Естественно, что не все цепочки заражённых данных ведут от источников к приёмникам. Есть несколько причин прерывания:
Однако следует помнить, что, если валидация происходит только при определённых условиях, это может быть даже более опасным случаем из-за иллюзии безопасности.
Способы валидации будут различаться для разных типов данных. Зависит от того, что мы ожидаем на вход: данные для SQL команды, какой-нибудь путь и т.п. Для защиты от SQLI, например, повсеместно рекомендуют использование параметризированных запросов.
String userName = Request.Form["userName"];
using (var command = new SqlCommand()
{
Connection = connection,
CommandText = "SELECT * FROM Users WHERE UserName = @userName",
CommandType = System.Data.CommandType.Text
})
{
var userNameParam = new SqlParameter("@userName", userName);
command.Parameters.Add(userNameParam);
using (var reader = command.ExecuteReader())
....
}В данном случае цепочка передачи taint-данных просто разорвётся при создании объекта типа SqlParameter. У анализатора нет информации о том, что он передаёт заражение или заражается. Как следствие, анализатор не будет считать переменную userNameParam заражённой. Исходное значение userName в создаваемую команду никак не попадает, поэтому предупреждение выдано не будет.
Естественно, что данные не попадают сразу из taint source в taint sink. Теоретически могут, конечно, но это уже совсем фантастический код. :) Скорее всего, после проникновения в приложение заражённые данные сначала распространяются различными путями и лишь затем могут попасть в taint sink. Причём способы заражения могут быть достаточно вариативными. Простые присвоения переменных — это самый очевидный кейс.
Соответствующий пример, на самом деле, уже был продемонстрирован ранее:
void Example()
{
var taintedVariable = Console.ReadLine();
TaintSink(taintedVariable);
}В этом фрагменте кода статус заражённого значения проставляется на вызов метода Console.ReadLine() и лишь затем передаётся переменной taintedVariable через операцию присвоения.
Заражения через переприсвоения аналогичны:
var taintedVariable = Console.ReadLine();
var taintedVariable2 = taintedVariable;Дополнительно мы рассмотрели ряд более интересных кейсов передачи заражения. Например, в строках заражение вполне себе передаётся через конкатенацию:
var shipCity = Console.ReadLine();
var resStr
= "select * from OrdersTable where ShipCity = '" + shipCity + "'";Анализируя операции конкатенации строк, мы проверяем, не является ли один из операндов заражённым. Если это так, выставляем соответствующий статус для всего выражения.
С таким же успехом данные могут передаваться через интерполированные строки:
var resStr = $"select * from UsersTable where Id = '{id}'";Принцип аналогичен — мы анализируем интерполируемые элементы и, если хотя бы один из них заражён, выставляем статус заражения на всё выражение.
Ещё один способ передачи заражения — через вызов методов. Здесь тоже есть, где разгуляться. :)
Один из вариантов — транслирование заражения из аргументов в возвращаемое значение. Например:
var resStr = String.Join(separator, nonTaintedStr, taintedStr);В результате выполнения этого кода при соединении строк заражение передастся от значения taintedStr в возвращаемое значение метода String.Join, а оттуда — в resStr.
Другой интересный кейс, с которым довелось столкнуться, — заражение объектов через аргументы метода, вызываемого у данного объекта. Типичный представитель подобных случаев — StringBuilder.
var sb = new StringBuilder();
sb.AppendLine(taintedStr);
var resStr = sb.ToString();Изначально sb не является заражённым, но становится таковым в результате вызова метода AppendLine, если taintedStr содержит заражённые данные. Теперь, когда объект sb является инфицированным, он может сам передавать заражение дальше. В нашем случае заражается переменная resStr в результате вызова метода ToString, транслирующего статус заражённости из вызывающего объекта в возвращаемое значение.
Естественно, все эти способы могут комбинироваться, а заражение затем может уйти вообще в другой метод — и такие случаи тоже неплохо бы обнаруживать.
Основное и самое обидное, с чем довелось столкнуться, — ограничения анализа значимых типов. Дело в том, что сейчас data-flow анализ в C# ограничен перечислениями и целочисленными типами, такими как int, byte, short и т.п. Из-за этого, как только в цепочке заражений встречается неизвестный значимый тип (какая-нибудь структура, например), она обрывается. Это большая точка для роста и доработок.
Вообще, так как это самая первая версия taint-анализа, уже есть некоторые идеи по доработкам и улучшениям. Будем допиливать со временем, в частности по мере разработки новых диагностических правил. Если столкнётесь с false positive срабатываниями или, наоборот, анализатор что-то не обнаружит, пожалуйста, напишите нам. Мы постараемся учесть эти случаи и поддержать их в будущем.
От общих механизмов переходим к их использованию. В принципе, здесь всё достаточно просто и общий алгоритм работы примерно одинаков.
Конечно, в диагностиках будет дополнительная логика, но общий принцип останется, думаю, примерно одинаковым.
Как я упоминал выше, первой диагностикой на taint-анализе стало правило поиска потенциальных SQL инъекций.
Не буду вдаваться в подробности, что такое SQL инъекция, какие виды бывают и т.д. В интернете достаточно информации на эту тему. Например, вот ссылочки на Википедию и на docs.microsoft.com. Обозначу только суть.
А суть очень проста и отлично ложится на базовую теорию taint-анализа, про которую мы рассуждали выше. Есть какой-то источник внешних данных (taint source). Данные в этом источнике поступают от пользователя, и он волен писать в них то, что ему вздумается. Эти данные приходят в приложение, перемещаются по нему и в результате попадают в SQL команду без проверки (sink). Если команда создаётся таким образом, что допустимыми являются любые данные от пользователя, он может скомпрометировать их так, чтобы выполнить произвольный запрос. Вот вам и инъекция.
Рассмотрим пример. Если вы уже гуглили запросы в духе "SQLI C#", то наверняка видели что-нибудь подобное:
private HttpRequest Request { get; set; }
void ProcessUserInfo()
{
using (SqlConnection connection = new SqlConnection(_connectionString))
{
....
String userName = Request.Form["userName"];
using (var command = new SqlCommand()
{
Connection = connection,
CommandText = "SELECT * FROM Users WHERE UserName = '" + userName + "'",
CommandType = System.Data.CommandType.Text
})
{
using (var reader = command.ExecuteReader())
....
}
}
}Проблема в том, что данные в userName поступают извне и подставляются в SQL команду без каких-либо проверок. Это чревато последствиями, если вместо адекватных данных в userName попадёт скомпрометированная команда. Например, если там будет строка ' OR '1'='1, то обработка данных запустится для всех элементов таблицы, а не одного пользователя.
Теперь разберём этот пример с точки зрения анализатора. Как он может обнаружить здесь угрозу возникновения SQLI?
В данном случае источником заражённых данных выступает свойство Request.Form (переменная Request имеет тип HttpRequest). Тип свойства Form — NameValueCollection, и именно экземпляр этого типа мы считаем заражённым. У объекта вызывается индексатор, который ретранслирует заражённые данные на всё выражение (возвращаемое значение индексатора): Request.Form -> Request.Form["userName"]. Так как инициализатор переменной userName заражён, она также становится заражённой.
Анализируем вызов конструктора типа SqlCommand, точнее инициализацию свойств. Нас интересует свойство CommandText. В нашем случае CommandText — sink. То есть при записи в него заражённых данных должна срабатывать диагностика. Поэтому анализируем правую часть присваивания: "SELECT * FROM Users WHERE UserName = '" + userName + "'". Ага, конкатенация строк. Мы помним, что, если хотя бы один операнд имеет заражение, оно передаётся на всё выражение. Как мы помним, userName заражён. Так что в результате конкатенации становится заражённым и всё выражение целиком. Выходит, значение, записываемое в CommandText, заражено, а это как раз то, что мы хотели проверить.
Taint анализ в описанном виде появился в релизе PVS-Studio 7.13. Там есть и диагностика по поиску возможных SQLI — V5608. Вы уже можете попробовать новую версию анализатора.
Безусловно, работы в этом направлении много. И как по улучшению общего механизма taint-анализа, так и по разработке диагностик, его использующих. Поэтому, если у вас есть какие-нибудь идеи по улучшению, смело пишите нам, возьмём себе на заметку.
Как обычно, приглашаю подписываться на мой Twitter-аккаунт. ;)