- PVSM.RU - https://www.pvsm.ru -

Обнаружение дефектов кода типа «Expression Issues» (CWE-569)

Настоящей статьей мы продолжаем серию обзоров, посвященных обнаружению уязвимостей в open-source проектах с помощью статического анализатора кода AppChecker [1].

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

Обнаружение дефектов кода типа «Expression Issues» (CWE-569) - 1

В международной классификации CWE данный тип дефектов известен как CWE-569: Expression Issues. К нему относятся различные ошибки в логических выражениях в коде программы. Частным случаем дефекта такого класса является дефект «Присваивание вместо сравнения».

Присваивание вместо сравнения (CWE-481: Assigning instead of Comparing) – дефект, как видно из названия, заключающийся в том, что вместо оператора сравнения в коде программы используется оператор присваивания. Подобная ошибка в коде ведет к некорректной работе программы и может привести к серьезной бреши в безопасности.

Страх совершения подобных ошибок довел того, что появилась так называемая нотация Йоды (Yoda notation), требующая написании константы или вызова функции слева от оператора сравнения: «7 == j» вместо привычного «j == 7». Давайте посмотрим код. Например, такой:

  pswd=GetText();
  if (consthash=hash(pswd)); //авторизовать пользователя

Предположим, что pswd – переменная, в которой хранится введенный пользователем пароль. В условном операторе должно происходить сравнение хэша от пароля с заранее заданным значением. Если эти значения совпадают, считается, что пароль совпал, и пользователь получает доступ. Однако в этом примере вместо оператора сравнения «==» стоит оператор присваивания «=». В этом случае переменной consthash присвоится значение hash(pswd), и поскольку эта операция выполнена успешно, программа посчитает, что условие выполняется, что, в свою очередь, приведет к авторизации пользователя, независимо от введенного им пароля.

Казалось бы, что данный пример достаточно искусственный и вряд ли может встретиться в серьезных программных продуктах. Однако, анализ opensource проектов показал, что этот дефект встречается довольно часто. Обычно такие дефекты возникают из-за невнимательности разработчика. Подобные дефекты можно обнаруживать с помощью сигнатурно-эвристического анализа.

Логично предположить, что существует и обратный дефект: сравнение вместо присваивания. Действительно, классификация CWE определяет этот тип дефектов, как CWE-482: Comparing instead of Assigning.

Далее приведем пример реального кода, в котором наоборот вместо операции присваивания используется операция сравнения.

if ($mValueCount == floor($mValueCount)) {
     ...
} else { 
     $mValueCount == floor($mValueCount);
     ... 
}

Как видно, по ошибке программиста выражения в условии и теле условного оператора идентичны, хотя во втором случае явно должен быть оператор присваивания. Этот фрагмент кода взят из библиотеки для работы с документами с помощью PHP – PHPExcel версии 1.8.1. Мы уведомили разработчиков об обнаруженном с помощью AppChecker дефекте, и уже выпущен патч, в котором этот дефект был исправлен (https://github.com/PHPOffice/PHPExcel/pull/710/files [2]).

Еще один пример такого типа дефектов был найден в библиотеке для работы с MP3, которая входит в состав некоторых CMS, в частности wordpress и написанной на языке PHP – getID3-1.9.10 [3]:

if (ord($frame_ownerid) === 0) {
     $frame_ownerid == '';
}

Как видно из этого фрагмента кода в теле условного оператора происходит сравнение, хотя по логике должно быть присваивание. Мы уведомили разработчиков об этом дефекте, и они оперативно этот дефект исправили (https://github.com/JamesHeinrich/getID3/pull/57/files [4]).

Тем не менее, класс «Дефекты в выражениях» не ограничивается этими двумя дефектами. Другим примером может являться дефект «Выражение всегда верно» (CWE-571: Expression is always true) и «Выражение всегда ложно» (CWE-570: Expression is always false). Как очевидно из названия, эти дефекты подразумевают написание условия в условном операторе, которое будет всегда верно/всегда ложно, что в свою очередь означает, что в условном операторе в данном участке кода нет никакой надобности.

В качестве примера рассмотрим фрагмент кода CMS с открытым исходным кодом Chamilo LMS 1.10.4 [5], написанной на языке PHP:

if ($row['item_type'] != 'dokeos_chapter' || $row['item_type'] != 'dokeos_module') {

Выражение в условном операторе верно всегда, поскольку для него необходимо, чтобы одно и то же значение 'item_type' было либо не равно 'dokeos_chapter', либо не равно 'dokeos_module'. Таким образом, единственным вариантом, при котором это выражение будет ложным – когда 'item_type' одновременно равен обоим, в общем случае, разным значениям. Разработчики были уведомлены о дефекте и оперативно его исправили (https://github.com/chamilo/chamilo-lms/pull/1109/files [6]). Как оказалось, в выражении должен стоять оператор «&&» вместо «||».

В качестве обратного примера, рассмотрим платформу для электронной коммерции с открытым исходным кодом OpenCart 2.2.0.0 [7]:

if ($chr == 252 && $chr == 253) {

Очевидно, что $chr не может быть одновременно равен 252 и 253. Этот дефект разработчики также устранили (https://github.com/opencart/opencart/pull/4231/files [8]). Как оказалось, в выражении должен стоять оператор «||» вместо «&&».

Так же к дефектам вида «Expression Issues» можно отнести случаи, когда обе части бинарного выражения идентичны (в частном случае это приводит к самоприсваиванию). Рассмотрим следующий фрагмент кода на Java:

if (s2.getClass() != s2.getClass()) return false;

Как видно из этого фрагмента кода, s2.getClass() сравнивается сам с собой, в результате чего условие всегда будет ложным. Вероятнее всего это просто опечатка программиста, но случай далеко не искусственный. Данный пример взят из google sageTV [9] – кроссплатформенной системы управления медиа-данными. Разработчики были уведомлены об этом дефекте и оперативно его исправили (https://github.com/google/sagetv/commit/93144762681a5f441ad011564ff8309095d9ca31 [10]).

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

Следующий пример на языке PHP взят из OpenEMR-4.2.1 [11]:

if ( !empty($obx24) || !empty($obx24) || !empty($obx25) )

Как видно из этого фрагмента кода, значение $obx24 проверяется дважды, хотя по логике в первом случае должно быть $obx23. Подобная невнимательность разработчика может привести к серьезным проблемам в работе программы. С этим согласны и разработчики OpenEMR – они оперативно исправили этот дефект, когда мы им о нем сообщили (https://github.com/openemr/openemr/pull/179 [12]).

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

В этой статье были рассмотрены дефекты типа «Expression Issues». Несмотря на простоту и кажущуюся банальность такого рода дефектов, они встречаются достаточно часто, как в open-source, так и в коммерческих проектах.

PS>
Бесплатную версию AppChecker [1] можно скачать с нашего сайта: https://file.cnpo.ru/index.php/s/o1cLkNrUX4plHMV [13]

Автор: Эшелон

Источник [14]


Сайт-источник PVSM.RU: https://www.pvsm.ru

Путь до страницы источника: https://www.pvsm.ru/java/237355

Ссылки в тексте:

[1] AppChecker: https://npo-echelon.ru/production/65/10920

[2] https://github.com/PHPOffice/PHPExcel/pull/710/files: https://github.com/PHPOffice/PHPExcel/pull/710/files

[3] getID3-1.9.10: http://getid3.sourceforge.net/

[4] https://github.com/JamesHeinrich/getID3/pull/57/files: https://github.com/JamesHeinrich/getID3/pull/57/files

[5] Chamilo LMS 1.10.4: https://chamilo.org/

[6] https://github.com/chamilo/chamilo-lms/pull/1109/files: https://github.com/chamilo/chamilo-lms/pull/1109/files

[7] OpenCart 2.2.0.0: https://www.opencart.com/

[8] https://github.com/opencart/opencart/pull/4231/files: https://github.com/opencart/opencart/pull/4231/files

[9] sageTV: http://sagetv.com/

[10] https://github.com/google/sagetv/commit/93144762681a5f441ad011564ff8309095d9ca31: https://github.com/google/sagetv/commit/93144762681a5f441ad011564ff8309095d9ca31

[11] OpenEMR-4.2.1: http://www.open-emr.org/

[12] https://github.com/openemr/openemr/pull/179: https://github.com/openemr/openemr/pull/179

[13] https://file.cnpo.ru/index.php/s/o1cLkNrUX4plHMV: https://file.cnpo.ru/index.php/s/o1cLkNrUX4plHMV

[14] Источник: https://habrahabr.ru/post/320398/?utm_source=habrahabr&utm_medium=rss&utm_campaign=best