Приветствую всех хабрачитателей. Помимо административной работы моей основной деятельностью является поиск различных уязвимостей. Чаще всего мой инструментарий представляет собой набор каких-то отладчиков, динамических анализаторов и прочего подобного. Но иногда приходится заниматься анализом исходного кода произвольной степени кривизны понятности. И это практически отдельный параллельный мир в области безопасности.
В этот раз нелегкая занесла меня в анализ исходного кода OpenJDK (посмотреть можно, например, на гитхабе). Начал с простого: получил журналы статического анализа кода и принялся их изучать. В общем смысле, журнал был довольно скучный — около 1500 предупреждений, 99% из которых — ложные срабатывания анализатора. Самое обидное, не смотря на то, что я наперед знаю: почти всегда более 99% предупреждений бесполезны, — все равно на каждое нужно потратить время для проверки не входит ли оно в тот самый 1%.
Один относительно интересный кейс я все же разберу. В лучших традициях блога PVS-Studio сначала покажу кусок кода и предложу найти проблему глазами.
static boolean setHuffTable(JNIEnv *env,
JHUFF_TBL *huff_ptr,
jobject table) {
jshortArray huffLens;
jshortArray huffValues;
jshort *hlensBody, *hvalsBody;
jsize hlensLen, hvalsLen;
int i;
// lengths
huffLens = (*env)->GetObjectField(env,
table,
JPEGHuffmanTable_lengthsID);
hlensLen = (*env)->GetArrayLength(env, huffLens);
hlensBody = (*env)->GetShortArrayElements(env,
huffLens,
NULL);
CHECK_NULL_RETURN(hlensBody, FALSE);
if (hlensLen > 16) {
/* Ignore extra elements of bits array. Only 16 elements can be
stored. 0-th element is not used. (see jpeglib.h, line 107) */
hlensLen = 16;
}
for (i = 1; i <= hlensLen; i++) {
huff_ptr->bits[i] = (UINT8)hlensBody[i-1];
}
(*env)->ReleaseShortArrayElements(env,
huffLens,
hlensBody,
JNI_ABORT);
// values
huffValues = (*env)->GetObjectField(env,
table,
JPEGHuffmanTable_valuesID);
hvalsLen = (*env)->GetArrayLength(env, huffValues);
hvalsBody = (*env)->GetShortArrayElements(env,
huffValues,
NULL);
CHECK_NULL_RETURN(hvalsBody, FALSE);
if (hvalsLen > 256) {
/* Ignore extra elements of hufval array. Only 256 elements
can be stored. (see jpeglib.h, line 109) */
hlensLen = 256;
}
for (i = 0; i < hvalsLen; i++) {
huff_ptr->huffval[i] = (UINT8)hvalsBody[i];
}
(*env)->ReleaseShortArrayElements(env,
huffValues,
hvalsBody,
JNI_ABORT);
return TRUE;
}Если кто-то хочет посмотреть сам на гитхабе, то вот ссылка на файл jdk/src/java.desktop/share/native/libjavajpeg/imageioJPEG.c. Функция начинается на 746 строке.
Хотя проблему можно увидеть на приведённом куске кода, для понимания видимой части проблемы нужно посмотреть еще на соседние заголовочные файлы.
Скрытый текст
Собственно, проблема в 46-й строке фрагмента выше или 791 строке на гитхабе.
Обратите внимание, что проверяется одна переменная hvalsLen, а меняется значение у другой - hlensLen, которая дальше по коду не используется. То есть если hvalsLen будет больше 256, то обращения в 49-й строке выйдут за ожидаемый предел.
Теперь расскажу, почему же это особенно плохо. Дело в том, что hvalsBody — это массив фиксированного размера из 256 элементов. Память под этот массив (ну точнее под структуру, полем которой является этот массив) выделяется на куче, так что потенциально все выглядит как переполнение буфера на куче (CWE-122).
Теперь начнем рассуждать логически и с лирическим отступлением. Когда я провожу триаж предупреждений, то присваиваю один из следующих возможных статусов: «потенциальная ошибка», «маловероятная ошибка», «не ошибка» и «ложное срабатывание анализатора".
Ложное срабатывание анализатора. Ровно то, что и написано — анализатор кода выдал какую‑то дичь (может парсер сломался или еще какая‑то логика заглючила). В общем, анализатор врет.
Не ошибка. Анализатор в общем‑то прав со своим предупреждением, но по каким‑то причинам это не является ошибкой.
Потенциальная ошибка. Так я пишу, когда вижу прям серьезный косяк в коде. Но я же осторожный — не всегда можно угадать, чем ошибка реально обернется, поэтому пишу все же «потенциальная».
Маловероятная ошибка. Тут интереснее всего — я вижу проблему в коде, но вместе с тем понимаю, что странный код находится в каком‑то довольно проходном месте. И тут получается следующая ситуация. Если бы проблема была серьезная, то код бы постоянно падал или плохо работал. Но в большинстве случаев падения не такие уж и частые. Значит тут есть какая‑то хитрость, например, даже если я не вижу каких-то ограничений в коде, они возможно где-то все же существуют.
В фрагменте кода, что был выше, явная ситуация «маловероятной ошибки»: если бы виртуальная машина падала из‑за кривой jpeg‑картинки, это было бы давно обнаружено. Значит, вероятно, проблема не такая уж и серьезная и, возможно, проверка на 256 лишняя и реально обеспечивается логикой, а не этой самой проверкой.
Я вышел в интернет с таким вопросом, поскольку изучение стандартов оставлю слабым духом людям. И нашел прекрасный код на питоне, который декодирует jpeg. Тут я точно уверовал, что это мой путь — анализ кода java-рантайма, написанного на плюсах, путем изучения питонячего кода.
Для моих целей все оказалось довольно просто — в начале файла 0xFFD8, в конце файла 0xFFD9, а между ними просто втыкаем блок с заголовком 0xFFC4, после которого идут два байта длины (что дает потенциальные 65 535, что прям хорошо так больше 256), байт описатель и далее там правильным образом сформированные массивы из этих 16 и 256+ байт.
IrfanView выдает вполне честную ошибку «Bogus Huffman table definition», видимо, что‑то знает. Но мы же проверяем не IrfanView, а OpenJDK. Поэтому качаем бинари рантайма, включаем плагины в vscode и пишем наш короткий шедевр.
try {
BufferedImage img = ImageIO.read(new File("C:\\tmp\\huffman.jpg"));
} catch (IOException e) {
System.err.println(e.getMessage());
}Результат запуска, к сожалению, не стартующий калькулятор, а всего лишь та же самая ошибка: Bogus Huffman table definition.
Дальше скучно и грустно: находим сообщение в гитхабе. А потом его использование в разных файлах.
В общем куча, так сказать, не переполнилась, простой вариант не сработал. Ну и ладно, я хотя бы нашел, где это проверяется в коде.
Внимательный программист может резонно заметить, что я пытался схалтурить загрузка картинки и вызов той функции с ошибкой в общем смысле не связаны. То есть, проведенная проверка, мягко говоря, покрывает не все случаи и, раз уж я взялся, надо проверять нормально. Реальный сценарий выглядит так: считали картинку, поработали с ней, и уже где-то после этого вызвалась вышеупомянутая функция.
С одной стороны — это крайне логично. При загрузке проверка выполнилась, а вот потом уже с картинкой что‑то сделали, как гипотетический пример — повернули, и вот так, внезапно, вышло, что таблица Хаффмана стала шире.
С другой стороны — если включить здравый смысл, то уязвимость становится проблемой только если ее можно стриггерить в некотором готовом коде. Если для триггера нужно еще написать прям специальный код на java, то это не особо имеет смысла. В общем, простые вещи делать можно, непростые – нельзя.
Какие же условия надо выполнить, чтобы была ошибка? hvalsLen должен быть больше 256 элементов, а этот массив берется как поле JPEGHuffmanTable_valuesID в классе объекта table, который передается как jobject table в функцию setHuffTable. Смотрим, кто вызывает рассматриваемую функцию: всего два варианта и оба в функции setHTables в этом же файле. Продолжаем строить колстек вверх, в вызовах аргумент table передается как элементы массивов jobjectArray DCHuffmanTables и jobjectArray ACHuffmanTables, которые оба являются аргументами функции setHTables. Весьма удобно, идем выше.
Функция setHTables вызывается уже в трех разных местах. Но названия у вызывающих функций весьма специфичны и явно указывают на то, что у них есть java-прообразы, а в качестве аргументов для setHTables передаются аргументы соответствующих функций без обработки. Так что сразу ищем native в java коде.
Java_com_sun_imageio_plugins_jpeg_JPEGImageWriter_writeTables
Java_com_sun_imageio_plugins_jpeg_JPEGImageWriter_writeImage
У этих функций интересующие нас аргументы имеют тип JPEGHuffmanTable. Конструктор этого класса встречает нас радостными проверками.
Значит, этим конструктором нельзя создать объект с полем values длиннее 256 элементов. Само поле закрытое, а его значение возвращается через копию, что исключает возможность внешней модификации.
Остался последний вариант испортить объект — второй конструктор, который заполняет поля без проверок.
Но тут и проверять особо нечего – конструктор закрытый, комментарий говорит, что это для внутреннего использования, да и вызывается он на вполне константных данных. Так что можно подвести итоги.
Общий вывод мы можем сделать такой: не смотря на явную опечатку в проверке, повлиять на безопасность openJDK она не может. Неправильно сформированная картинка пройдет несколько проверок при загрузке, которые не позволят обратиться за пределами выделенной памяти. Никакие корректные взаимодействия в java не позволят нам создать объект, обрабатывающийся в этой функции, чтобы он реализовал выход за пределы буфера.
А вот и все. Я же обещал приключение на 20 минут.
Ну и немного пустословия в конце статьи. А сообщил ли я разработчикам, а заслал ли я им патч, пулреквест или еще что-то? Неа. На это есть даже несколько причин. Фактически, ошибка не заслуживает особого внимания, это раз. Я не знаю что лучше в данном случае – поправить проверку или вообще ее убрать, это два. По сайту OpenJDK я не понял, куда будет правильно вообще об этом написать, это три. Мне было лень, это четыре.
