Поиск и исправление багов в исходниках PHP

в 12:59, , рубрики: bugs, open source, php, Анализ и проектирование систем, Блог компании Mail.Ru Group, никто не читает теги, Тестирование IT-систем

Поиск и исправление багов в исходниках PHP - 1

Честно предупреждаю: воспринимайте этот текст с определённой долей скептицизма. Я лишь недавно начал знакомство с внутренностями PHP, но хотел бы рассказать вам о том, что творится за кулисами бага #75237.

Обнаружение бага

Всё началось с поиска строк кода для покрытия на митапе Chicago PHP UG, который был частью PHP TestFest 2017.

Я нашёл непокрытую строку с помощью сайта gcov для ext/json/json_encoder.c. Попытался написать код на PHP, который обработал бы эту строку.

Поиск и исправление багов в исходниках PHP - 2

Попытался и так и эдак, но ничто не сработало, и внезапно я посредством этого кода ненароком инициировал segfault:

<?php
class Foo implements JsonSerializable {
  public function jsonSerialize() {
    return new self;
  }
}

# Segfault!
var_dump(json_encode(new Foo));

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

Чтобы узнать, какие версии PHP подвержены этому багу, я создал сниппет 3v4l и увидел, что все активно поддерживаемые версии генерируют segfault.

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

<?php
class Foo implements JsonSerializable {
  public function jsonSerialize() {
    return $this;
  }
}

# Works fine
var_dump(json_encode(new Foo));

Отправка отчёта о баге

Обо всех багах в PHP нужно сообщать посредством bugs.php.net. Хотя я и пытался патчить баг самостоятельно, мне всё же пришлось создать отчёт, на который можно ссылаться в патче.

Так что я заполнил форму — и возник баг #75237.

Написание патча

Теперь самое трудное. Как начать патчить эту ошибку?

Обновление php-src и перекомпилирование

Сначала я получил все предыдущие изменения из основного Git-репозитория, чтобы быть уверенным, что я работаю с наиболее свежей версией php-src. Я удалённо вызвал команду upstream, указывающую на основной репозиторий php-src на GitHub.

$ git fetch upstream
$ git checkout master
$ git rebase upstream/master master

Затем создал новую ветку master для патча и назвал её в честь ID бага.

$ git checkout -b bug75237

Теперь нужно перекомпилироваться из исходного кода. Не буду вдаваться в подробности, у меня есть другая статья, где подробно описано, как компилировать PHP из исходного кода.

Когда я перекомпилирую PHP, я запускаю bash-скрипт под названием config, который я храню вне основной папки php-src. Он удаляет все существующие скомпилированные бинарники и прочие не находящиеся под версионным контролем файлы, собирает конфигурационный скрипт и запускает конфигурирование с нужными мне флагами, включёнными по умолчанию.

Вот мой скрипт, если захотите сделать для себя что-то подобное:

#!/bin/sh

make distclean
./vcsclean
./buildconf
./configure --enable-maintainer-zts 
        --enable-debug 
        --enable-cli 
        --with-zlib 
        --enable-mbstring 
        --with-openssl=/usr/local/opt/openssl 
        --with-libxml-dir=/usr/local/opt/libxml2 
        --with-sodium 
        "$@"

Почему здесь такие странные пути? Я работаю на Mac с некоторыми зависимостями, установленными с Homebrew, поэтому кое-где ради некоторых расширений пути ведут в /usr/local/opt.

Так что я просто запустил свой конфигурационный скрипт, а затем скомпилировал всё с помощью make.

$ ../config && make -j9

По адресу sapi/cli/php создался бинарник, после чего я проверил версию PHP и запустил приводящий к segfault код, чтобы удостовериться, что мастер-ветка php-src still всё ещё содержит баг.

$ sapi/cli/php --version
$ sapi/cli/php json_encode.php

Да! Опять segfault. Теперь воспользуемся отладчиком, чтобы пройти по С-коду, который строка за строкой исполняет наш PHP-скрипт.

Выполнение GDB

Для отладки кода я использовал GDB. Он спроектирован для компилируемых языков вроде С и не работает с обычными PHP-файлами. Но PHP написан на С, так что с помощью gdb --args можно заставить GDB выполнить скомпилированный бинарник с PHP-скриптом.

$ gdb --args sapi/cli/php json_encode.php

Запустив GDB, я установил контрольную точку для паузы исполнения программы, а затем прошёл по каждой строке С-кода. О подробностях я рассказал в вышеприведённом видео, но если вкратце: оказалось, что всё дело в функции php_json_encode_serializable_object(), поэтому я установил здесь контрольную точку и запустил программу с помощью run.

> break php_json_encode_serializable_object
> run

Когда GDB доходит до контрольной точки и ставит исполнение кода на паузу, можно написать:

  • n и перепрыгнуть на следующую строку,
  • s и перейти в область видимости / фрейм следующей строки.
  • c и продолжить выполнение до следующей контрольной точки.

После ввода команд я понял, что код входит в бесконечную рекурсию между php_json_encode_zval() и php_json_encode_serializable_object(). Это приводит к переполнению стека и segfault, потому что у программы кончается память.

Меня интересовало выражение if внутри php_json_encode_serializable_object().

if ((Z_TYPE(retval) == IS_OBJECT) &&
        (Z_OBJ(retval) == Z_OBJ_P(val))) {
        //
} else {
        //
}

Там обрабатывался случай, когда возвращаемым jsonSerialize() значением является тот же экземпляр его самого, но никто не ловил ситуацию, когда возвращался новый экземпляр.

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

if ((Z_TYPE(retval) == IS_OBJECT) &&
        // Make sure the objects are not the same instance
        (Z_OBJ(retval) != Z_OBJ_P(val)) &&
        // Check that the class names of the objects are the same
        zend_string_equals(ce->name, Z_OBJCE(retval)->name)) {
        // Throw an exception
        zend_throw_exception_ex(NULL, 0, "%s::jsonSerialize() cannot return a new instance of itself", ZSTR_VAL(ce->name));

        // Garbage collection
        zval_ptr_dtor(&retval);
        zval_ptr_dtor(&fname);
        PHP_JSON_HASH_APPLY_PROTECTION_DEC(myht);

        // Return fail
        return FAILURE;
} else if ((Z_TYPE(retval) == IS_OBJECT) &&
        (Z_OBJ(retval) == Z_OBJ_P(val))) {
        //
} else {
        //
}

Я сделал много проб и ошибок, чтобы прийти к этому варианту, и мне очень помог сайт phpinternalsbook.com, особенно при работе со zval.

Обновление: Сара Голмон, долгое время являющаяся ключевым разработчиком PHP и одним из релиз-менеджеров PHP 7.2, дала замечательный комментарий к моему патчу: «Если пытаешься просто сопоставить класс, то можешь сравнить только ce. Так будет корректнее, чем сравнивать имена, и (номинально) более эффективно».

Благодаря предложению Сары мы можем изменить строку с zend_string_equals() в выражении if:

if ((Z_TYPE(retval) == IS_OBJECT) &&
        (Z_OBJ(retval) != Z_OBJ_P(val)) &&
        // This line changes
        ce == Z_OBJCE(retval)) {
        // Same as above
} else if ((Z_TYPE(retval) == IS_OBJECT) &&
        (Z_OBJ(retval) == Z_OBJ_P(val))) {
        //
} else {
        //
}

Тестирование

Прежде чем применить патч, нужно написать тест, подтверждающий, что ошибка действительно исправлена.

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

$ vi ext/json/tests/bug75237.phpt
--TEST--
Bug #75237 (jsonSerialize() - Returning new instance of self causes segfault)
--SKIPIF--
<?php if (!extension_loaded("json")) die("skip ext/json required"); ?>
--FILE--
<?php
class Foo implements JsonSerializable {
  public function jsonSerialize() {
    return new self;
  }
}

try {
  var_dump(json_encode(new Foo));
} catch (Exception $e) {
  echo $e->getMessage();
}
?>
--EXPECT--
Foo::jsonSerialize() cannot return a new instance of itself

Я запустил тест и проверил его прохождение, а затем прогнал все тесты в ext/json, чтобы удостовериться, что ничего не сломал.

$ make test TESTS=ext/json/tests/bug75237.phpt
$ make test TESTS=ext/json/tests/

Все тесты были пройдены, так что я был готов к применению патча.

Отправка PR и обновление бага

Затем я залил патч в свой форк на GitHub...

$ git add ext/json/json_encoder.c ext/json/tests/bug75237.phpt
$ git commit -m "Fix bug 75237 - (jsonSerialize() - Returning new instance of self causes segfault)"
$ git push origin 

… и создал PR в основном репозитории php-src.

Наконец, я вернулся к своему отчёту об ошибке и обновил его ссылкой на только что созданный PR.

Ожидание отклика и внесение изменений

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

В конце концов Niki P закрыл PR, потому что мы выявили более глубокую, более общую проблему, связанную с PHP. У этого языка нет общей защиты от переполнения стека, а значит, segfault может быть создан во многих других контекстах.

Вот как об этом высказалась Сара:

Удачи!

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

Надеюсь, вам понравился рассказ о моём приключении с багом. Желаю удачи в поисках и исправлении собственных багов в исходном коде PHP!

Автор: AloneCoder

Источник

Поделиться

* - обязательные к заполнению поля