В жизни каждого разработчика наступает момент, когда он вырастает до проведения code review своих коллег. Процесс формулирования замечаний к коду весьма тонок и здорово прокачивает навыки: как дипломатию, так и чисто человеческие эмпатию и милосердие.
С тех пор как этот момент настиг и меня, я веду коллекцию самых выдающихся багов, с которыми мне посчастливилось столкнуться. Даже не для того, чтобы о них рассказывать, нет… Просто чтобы не забывать, глядя на код коллеги в очередном MR, какую дичь порой творил (и продолжаю творить) я сам.
Есть баги, за которые и спустя несколько лет испытываешь чувство стыда и вины. А есть те, которые вполне можно считать такими же предметами гордости, как и реализованные фичи. Некоторыми жемчужинами своей коллекции позволю себе поделиться ниже.
Семь раз отмерь…
Начну, пожалуй, с бага, за который мне до сих пор искренне стыдно.
В JSA (сканирующем ядре PT Application Inspector) реализован механизм анализа, который позволяет переключать режим интерпретации кода на сложных участках в более примитивный. Он дает менее точные результаты, но значительно сокращает время сканирования. Если сильно упростить, в основе этого механизма лежит расчет цикломатической сложности symbolic-формул, описывающих какой-либо набор путей в графе потока данных. Когда сложность превышает заданный порог на очередной ноде, она начинает обрабатываться иначе. Это сводит символьное выполнение кода к тривиальному taint-анализу, который менее затратен по времени и памяти.
Реализацию этой фичи нельзя было назвать тривиальной. Расчет сложности требовалось добавить в конструкторы всех типов нод деревьев выражений. Затем нужно было реализовать ее учет и правильную обработку во всех базовых визиторах. На тот момент их уже было больше десятка, и далеко не в каждом логика методов могла похвастаться простотой.
Закончив экспериментальную часть, я приступил к тестированию на реальных сканируемых проектах и почти сразу поймал странное поведение. Периодически сложность будто вообще не учитывалась на некоторых участках фактически сложного кода. Поймать эту ситуацию в отладчике было непросто, поскольку сложность рассчитывалась на одном этапе жизненного цикла ноды, а учитывалась — совершенно на другом (еще и в других частях проекта). Да и проявлялось это в настолько больших сканируемых приложениях, что пройти под отладчиком весь процесс их интерпретации было весьма затруднительно.
Первый день коллеги смеялись, наблюдая за моими тщетными попытками разобраться, в чем дело. Им приходилось выслушивать все, что я думаю о нашей кодовой базе, визиторах, отладчике и родне по женской линии неопределенного круга лиц, имевших отношение к разработке платформы .NET («знал бы за что, вообще убил бы» ©). На второй день мне искренне пытались помочь. Мы вместе тупили в код, не понимая, из-за чего реализованный функционал периодически не срабатывает. И ведь никто, совершенно никто не увидел двух допущенных мной багов, которые, даже чисто теоретически, нельзя было не заметить... На третий день, отчаявшись, я добавил в лог выгрузку всех значений сложности, которые встречались во время сканирования, и запустил анализатор на большом проекте. Каково же было мое изумление, когда в логе я увидел… отрицательные числа. Сложность никак не могла быть отрицательной! Я до сих пор искренне радуюсь тому, что почему-то использовал для нее тип long, а не ulong. Иначе мы бы так и не разобрались в причинах бага и не зарелизили этот механизм.
А причины-то были просты. По глупой ошибке я использовал умножение вместо сложения при расчете сложности наиболее критичного типа узлов, где она и без того росла как на дрожжах на более-менее больших приложениях. Но это было ничто по сравнению со второй допущенной ошибкой. Заботливо обернув всю арифметику расчета сложности в проверки на переполнение, я по неведомой причине использовал ключевое слово unchecked вместо checked. И тем самым отключил контроль переполнения... Сложность на отдельных частях кода вырастала слишком быстро, становилась отрицательной и спокойно проходила проверку на превышение порога. На самом деле, она росла настолько быстро, что иногда проскакивала все множество отрицательных значений, к моменту проверки становилась положительной и зачастую не превышала порог (что я и наблюдал в отладчике на больших приложениях). В итоге первая реализация фичи попала в репозиторий в следующем виде (см. рис. 1).
Сейчас она выглядит иначе: проверки переполнения убраны в методы-расширения, которые логируют каждый факт выброса исключения, а для значения сложности используется ulong. От последнего у меня до сих пор дергается глаз, когда вспоминаю про три чудесных дня, проведенных за отладчиком.
Пляшущие тесты
В одну замечательную пятницу, под вечер, черт дернул меня нанести очередную непоправимую пользу модулю анализа JavaScript-кода JSA. Речь шла об улучшении поддержки детектирования NoSQL-инъекций в запросах MongoDB. Что-то вроде { $where: { blablabla: request.a } }. В JSA эта строка интерпретируется в полноценное дерево объектов, которые описывают symbolic-модель запроса — со всеми условиями достижимости значений полей и прочими семантическими свойствами. Несмотря на наличие столь мощной модели, поддержка таких запросов у нас была сделана для весьма частного случая. У объекта, который соответствовал запросу, запрашивалась symbolic-формула значения поля $where. Если в ней присутствовали выражения, потенциально контролируемые атакующим, все это хозяйство передавалось в Большой Адронный Решатель aka «обертка над SMT-солвером». Там и происходил процесс детектирования уязвимости. Идея улучшения заключалась в том, чтобы решать в анализаторе более общую задачу и обходить все дерево объектов запроса, отправляя формулы подозрительных значений решателю.
Казалось бы, что могло пойти не так? Пришлось чуть повозиться, но за пару часов я управился. С удовлетворением отметил, что в одном из уязвимых проектов, на которых мы тестируем свой движок, количество детектируемых уязвимостей (не фолзов) выросло более чем в полтора раза. Перед коммитом требовалось добавить функциональные тесты на вновь обнаруженные уязвимости, но уже была поздняя ночь. Чтобы окончательно разделаться с этой задачей, я решил выкроить часик субботы. И правильно сделал: впоследствии выяснилось, что возьмись я за тесты сразу, спать бы уже не лег.
Субботним утром я решил не откладывать задачу в долгий ящик и прямо за чашкой кофе набросал тесты на недостающие уязвимости. Для функционального тестирования мы используем собственный простенький движок. Он построен поверх NUnit: сообщаешь в тесте путь к проекту, который надо просканировать, описываешь на fluent-expression DSL шаблоны уязвимостей и, собственно, все. После прогона анализа в результатах выводится разница между ожидаемым и полученным множеством уязвимостей, если она есть. Движок был простой и неприхотливый: важен был даже порядок описания шаблонов уязвимостей. Также требовалось совпадение с порядком, в котором анализатор обнаруживал эти уязвимости.
Добив руками шаблоны для новых уязвимостей, я запустил тест и... фейл. Где-то в середине результата сканирования встретилась не та уязвимость, шаблон которой был описан. Ладно, думаю, не проснулся еще — перепроверю. Так и есть: перепутал позиции для двух уязвимостей, из-за чего их порядок в шаблоне отличался от порядка в результатах скана. Ок, поправил шаблон. Снова запускаю тест, и снова фейл — по той же причине, но уже с другой уязвимостью. Ничего не меняя, запускаю еще раз. Опять фейл, опять та же причина и... опять с ДРУГОЙ уязвимостью. WTF?!!
Многократный прогон одного и того же теста показал, что порядок найденных уязвимостей каждый раз менялся. Причем это происходило только с теми уязвимостями, которые начали детектироваться благодаря моим изменениям. Интуиция подсказывала, что придется лезть в отладку недр анализатора. Чтобы вы понимали: там есть такие места, куда даже с отладчиком можно влезать только после 21 года и предварительно записавшись на прием к психологу. Нет, с качеством кода там все в порядке, просто он такой же сложный, как и сама предметная область.
Отладкой анализатора было удобно заниматься через запуск его консольной версии. В этом случае доступен подробный лог работы, включая байт-код, интерпретируемый в ходе анализа (по ряду причин модули анализа динамических языков работают в JSA именно с ним, а не с исходниками). Чтобы вы понимали, выглядело это примерно так (см. рис. 2).
Запускаю отладчик один раз, второй, третий и вижу, что порядок уязвимостей не меняется! Тут же начинаю тесты и наблюдаю изменение порядка уязвимостей при каждом новом прогоне. Ок, любопытный факт — похоже, отлаживаться придется на тест-раннере без подробного лога перед глазами. Остаток субботы и половина воскресенья ушли на безуспешные попытки понять логику бага и хотя бы приблизительное место его локализации. Уже много лет я не оказывался в ситуации, когда исчерпаны все предполагаемые причины ошибки и ни одна из них не нашла подтверждения. А порядок следования уязвимостей все менялся и менялся. В какой-то момент мне даже стало казаться, что они подпрыгивают в результатах сканирования вверх-вниз, издеваясь надо мной в извращенном танце. Задумался о записи к психологу…
К обеду воскресенья я случайно выяснил, что проблему решает предварительная сортировка полученных уязвимостей в тестовом движке. Но это борьба со следствиями, а хотелось бы понять причины. Вполне возможно, из-за моих изменений в логике работы анализатора начала твориться неведомая дичь.
К вечеру я сдался и отвлекся на домашние дела, чтобы хоть как-то успокоиться. И тут краешком мозга я зацепился за воспроизводимость бага в разных условиях. Что отличает запуск прогона тестов от консоли и UI PT AI? Ответ пришел сам собой: отличаются версии рантайма. Наш проект в тот период соответствовал .NET Standard 2.0, потому что в отдаленных планах было использовать его под Linux и macOS. Но по ряду причин из-под UI он запускался под классическим дотнетом. Под ним же я запускал и отладку консольной версии в IDE. А вот тестовый движок целиком и полностью работал под .NET Core (не спрашивайте, пожалуйста, почему так получилось). Внезапно в голове всплыла проблема с DDoS на хеши ключей в словарях HTTP-запросов ASP.NET в начале 2010-х. Не могла ли реализованная тогда рандомизация хешей не работать в классическом .NET, но работать в Core?
Теперь, зная, что именно нужно гуглить, я моментально нашел несколько статей, которые дали мне ответы на все вопросы. Действительно, в классическом .NET механизм рандомизации хешей по умолчанию отключен, а вот в .NET Core является единственно возможным. Он гарантирует идентичность хешей только в пределах конкретного аппдомена. Иными словами, при каждом запуске приложения одна и та же строка будет иметь разные хеши. Следовательно, будет отличаться как порядок элементов в словарях со строковыми ключами, так и порядок обхода дерева NoSQL-запроса и обнаружения уязвимостей.
На тот момент пришлось оставить костыль с сортировкой результатов сканирования, полученных при прогоне тестов. Сейчас тестовый движок уже не требует определенного порядка выдачи результатов сканирования.
Дело о пропавшей переменной
Другой баг, который выбил меня из работы почти на целый день, был связан с одним из старых ядер PT AI. На тот момент наши сканирующие legacy-ядра для PHP, Java и C# доживали свои последние дни, поскольку мы переходили на JSA. В свое время в них был реализован механизм runtime virtual patching (RVP). Он позволял выгружать результаты анализа проекта в отчет для PT Application Firewall v3, состоявший из symbolic-формул уязвимых трасс кода. Специальный модуль PT AF, в свою очередь, моделировал поведение приложения при HTTP-запросах к уязвимым эндпоинтам и блокировал те, которые могли привести к эксплуатации уязвимости. Эдакий «RASP наоборот», в котором вместо встраивания сенсоров PT AF в приложение куски самого приложения встраивались в PT AF.
В тот злополучный день ко мне пришли коллеги из команды разработки PT AF с ошибкой загрузки отчета RVP, построенного по тестовому .NET-приложению. Он представлял собой XML-файл, который при загрузке валидировался на соответствие схеме, зафиксированной на стороне как PT AI, так и PT AF. Именно эта валидация стала завершаться с ошибкой после очередного релиза PT AI. Поиск причины бага осложнялся тем, что загрузка RVP-отчета тестировалась на стороне PT AF явно вручную и от случая к случаю. Из-за этого было непонятно, после каких изменений в PT AI начались проблемы. Но это была не самая большая головная боль. В попытке понять, что именно было не так, я потратил несколько часов только на сравнение полученного отчета с кодом анализируемого приложения. В ряде случаев в формулах вместо оператора логического неравенства внезапно появлялся оператор логического «И» с теми же аргументами. Еще какое-то время ушло на поиск в модуле анализа C# реализации построения отчета. Оказалось, что после того, как я залил первоначальную реализацию RVP, ребята провели рефакторинг архитектуры этой части проекта. Как следствие — функциональность RVP уехала в совершенно другое место и претерпела существенные изменения в кодовой базе.
Как выяснилось позже, этот рефакторинг и породил проблему, причем весьма забавным образом. Закончив с архитектурными улучшениями, парни заодно ощутимо упростили кодовую базу трансформера-визитора, который преобразовывал symbolic-формулы в узлы XML-отчета. Многие блоки кода были перенесены из одних методов визитора в другие. В результате одного из таких переносов оказалась потеряна инициализация переменной, которая отвечала за тип логической операции. Однако это не привело к ошибке времени компиляции: переменная с таким же именем по роковому совпадению уже была объявлена в этом методе (чуть выше в коде). Поскольку переменная была перечисляемого типа, она сразу получала значение по умолчанию, равное первому элементу перечисления, которым и был оператор логического «И». К сожалению, изначально я не покрыл этот код тестами.
Коммит с исправлением бага, который отнял у меня столько часов, состоял ровно из одной строки (не считая тестов) — см. рис. 3.
Поймай меня, если сможешь
Закончу багом, удивительным сразу по двум причинам. Во-первых, он появился в проекте сам собой — как результат пересечения нескольких вполне корректных изменений в кодовой базе, которые вносились с интервалом в несколько лет. Во-вторых, это единственный нетривиальный баг на моей памяти, который я нашел, отлаживая код в уме — без помощи отладчика.
Пару лет назад я лениво прогонял функциональные тесты, чтобы оценить разницу во времени сканирования от запуска к запуску. В одном из проектов всплыла «лишняя» уязвимость, которая проявлялась довольно редко. Поскольку прогоны осуществлялись на релизной ветке, я решил разобраться, в чем дело, и попал в замечательный квест почти на две недели (на 2–3 часа в день).
Серия дальнейших экспериментов показала, что баг проявляется только под OS X примерно один раз на сотню запусков. При этом, если к процессу подключен отладчик, в релизной и отладочной конфигурации ошибки не будет. Если вы работали с параллельными и конкурентными вычислениями, то наверняка догадываетесь, к чему все идет. Когда нет возможности использовать отладчик, на помощь приходит логирование нужной информации. Но вот незадача: добавление логирования (даже переключение уровня логирования в более подробный режим) также препятствовало воспроизведению бага. Оставалось одно: строить гипотезы, откуда могло появиться такое поведение, и кропотливо проверять их, глядя на код и интерпретируя его в уме.
Не буду утомлять подробностями о том, насколько увлекательным и освежающим мозг занятием была такая «отладка». Скажу лишь, что еще раз столкнуться с проблемами, связанными с параллельными вычислениями в нескольких потоках, мне бы не хотелось. В первую очередь я проверил гипотезы о гонках за ресурсы — это ничего не дало. Здесь стоит пояснить, как именно в JSA работает механизм многопоточного сканирования.
Он основан на технологии, схожей с линуксовыми форками. Когда анализатор обнаруживает в коде очередную точку входа (например, обработчик роута HTTP-запроса), он создает в памяти собственную копию. Это позволяет не воспроизводить заново модель памяти, актуальную на момент старта анализа точки входа. Далее на этой копии запускается сканирование точки входа в отдельном потоке. По умолчанию при создании форка все объекты анализатора рекурсивно копируются по значению. Однако можно разметить в коде нужные классы специальным атрибутом. Тогда объекты этих классов будут скопированы по ссылке и разделятся между потоками. Благодаря этому в коде как на ладони будут видны все места, где могут возникнуть гонки. Именно поэтому соответствующие гипотезы получилось проверить достаточно быстро. Тем не менее было очевидно: проблема связана именно с многопоточностью, поэтому нужно искать другие кейсы, где она может проявиться. В результате нащупать багу удалось скорее интуитивно, чем каким-то формальным способом.
В JSA есть такая штука, как стрингификатор. Он получает на вход symbolic-формулу куска графа потока данных и преобразует его в корректный код на языке сканируемого проекта. Зачем это нужно? Например, для формирования человекочитаемых результатов сканирования (доп. условия существования уязвимости в PT AI и плагинах IDE — дело его методов) и записей формул в логе. Или же для построения кода выражения, которое необходимо выполнить на реальном интерпретаторе языка. В основе стрингификатора лежит стандартный StringBuilder, в котором строится результирующий код. Давным-давно, когда то, что легло в основу JSA, было модулем анализа C#-кода, разработчики ограничили максимальную длину конструируемого кода 64 тысячами символов (см. рис. 4).
Это имело смысл. С одной стороны, код большего размера нельзя нормально отобразить в UI и отчетах о сканировании. С другой — солвер не сможет быстро его обработать. В крупных проектах такие строки могут запросто сожрать всю память.
А еще в JSA есть объекты. Это класс нод symbolic-формул, обозначающих ссылку на объекты в модели памяти интерпретатора JSA. Когда начинает работать стрингификатор, все объекты с известным состоянием заменяются в формулах на object-литералы, которые его описывают. Но чаще встречаются объекты с неизвестным состоянием: как минимум объекты фреймворка, внешних зависимостей и стандартной библиотеки языка. Стрингификатор преобразует их в код Object<N> (где N — числовой идентификатор объекта). Этот код не может быть выполнен реальным интерпретатором языка, но дает более читаемые результаты в отчетах о сканировании. Идентификаторы у объектов сквозные: счетчик числа созданных объектов разделяется между потоками, а при создании нового объекта инкрементируется и становится его идентификатором. Отмечу, что даже на средних проектах объектов может быть много — десятки и сотни тысяч.
Допустим, в ходе скана были параллельно запущены два потока с форками под разные точки входа — A и B. Первым почти всегда начинал свою работу поток A, а следом за ним — B. Но в одном случае из ста звезды на небе (а на самом деле внутреннее состояние планировщика потоков) сходились так, что раньше стартовал поток B. При этом номера объектов в обоих потоках отличались от предыдущего случая. Как результат — стрингификация одной из формул потока A превышала заданный порог StringBuilder (из-за длинных чисел в идентификаторах объектов), и выбрасывалось исключение. Поскольку это происходило в ходе решения солвером уравнения с этой формулой, он относил кейс в разряд «я не справился, пусть лучше человек проверит». Солвер рапортовал об уязвимости с доп. условием (включавшим нерешенную формулу, которая в первом случае решалась без исключений в false) и принимал решение о том, что уязвимости здесь нет.
Исправление бага было слишком обширным, чтобы приводить его здесь. Если кратко: мы «попросили» реальный интерпретатор языка размечать все ноды флагом исполнимости. Если на момент решения уравнения формула содержала такие ноды (к которым относились в том числе все Object[N]), до их стрингификации дело просто не доходило (см. рис. 5).
***
Напоследок хочу сказать, что существенно сократить потенциальные размеры коллекции багов мне помог простой принцип. Посыл «Работает — не трогай!» я нахожу весьма ущербным — не предполагающим развития и роста над собой. Куда перспективнее, на мой взгляд: «Заработало? Разберись почему!» :-)
