Заметка про проверку PHP
PHP — скриптовый язык программирования общего назначения, интенсивно применяемый для разработки веб-приложений. В настоящее время поддерживается подавляющим большинством хостинг-провайдеров и является одним из лидеров среди языков программирования, применяющихся для создания динамических веб-сайтов.
В случае с компиляторами и интерпретаторами к исходному коду и тестированию, как правило, предъявляются повышенные требования качества и надёжности. Тем не менее, в исходном коде интерпретатора PHP нашлись подозрительные места.
В данной статье будут рассмотрены результаты проверки интерпретатора PHP, полученные с помощью PVS-Studio 5.18.
Одинаковые условные выраженияV501 There are identical sub-expressions '! memcmp («auto», charset_hint, 4)' to the left and to the right of the '||' operator. html.c 396 static enum entity_charset determine_charset (char *charset_hint TSRMLS_DC) { … if ((len == 4) /* sizeof (none|auto|pass) */ && //<== (!memcmp("pass", charset_hint, 4) || !memcmp("auto", charset_hint, 4) || //<== !memcmp("auto", charset_hint, 4))) //<== { charset_hint = NULL; len = 0; } .... } Условное выражение содержит вызов функций 'memcmp' с одинаковыми параметрами. Комментарий /* sizeof (none|auto|pass) */ подсказывает нам, что в одну из функций необходимо передать значение «none».Всегда ложное условие V605 Consider verifying the expression: shell_wrote > — 1. An unsigned value is compared to the number -1. php_cli.c 266 PHP_CLI_API size_t sapi_cli_single_write (…) { … size_t shell_wrote; shell_wrote = cli_shell_callbacks.cli_shell_write (…); if (shell_wrote > -1) { //<== return shell_wrote; } .... } Данное сравнение является явной ошибкой. '-1' преобразуется в максимальное значение типа 'size_t', поэтому условие всегда будет ложным, и вся проверка сошла на нет. Возможно, переменная 'shell_wrote' раньше имела знаковый тип, но после рефакторинга не учли особенности операций с беззнаковыми типами.Некорректное условие V547 Expression 'tmp_len >= 0' is always true. Unsigned type value is always >= 0. ftp_fopen_wrapper.c 639 static size_t php_ftp_dirstream_read (…) { size_t tmp_len; … /* Trim off trailing whitespace characters */ tmp_len--; while (tmp_len >= 0 && //<== (ent->d_name[tmp_len] == '\n' || ent→d_name[tmp_len] == '\r' || ent→d_name[tmp_len] == '\t' || ent→d_name[tmp_len] == ' ')) { ent→d_name[tmp_len--] = '\0'; } … } Тип 'size_t', являясь беззнаковым, позволяет индексировать максимальное количество элементов массива для текущей разрядности приложения. Проверка (tmp_len >= 0) является некорректной. В худшем случае декремент может привести к переполнению индекса и обращению к памяти за границей массива. Скорее всего, код выполняется верно благодаря дополнительным условиям и корректным исходным данным, но опасность «зацикливания» или выхода за пределы массива тут присутствует.Разность беззнаковых чисел V555 The expression 'out_buf_size — ocnt > 0' will work as 'out_buf_size!= ocnt'. filters.c 1702 static int strfilter_convert_append_bucket ( { size_t out_buf_size; … size_t ocnt, icnt, tcnt; … if (out_buf_size — ocnt > 0) { //<== .... php_stream_bucket_append(buckets_out, new_bucket TSRMLS_CC); } else { pefree(out_buf, persistent); } .... } Возможно, ветвь 'else' будет выполнять реже, чем следовало бы, т.к. разность беззнаковых чисел почти всегда будет больше нуля. Исключением будет являться равенство операндов: в этом случае условие лучше переписать на более информативный вариант.Разыменование указателя V595 The 'function_name' pointer was utilized before it was verified against nullptr. Check lines: 4859, 4860. basic_functions.c 4859 static int user_shutdown_function_call(zval *zv TSRMLS_DC) { .... php_error(E_WARNING, "....", function_name->val); //<== if (function_name) { //<== STR_RELEASE(function_name); } .... } Проверка указателя после разыменования всегда выглядит подозрительно. В случае реальной ошибки может привести к краху программы.Аналогичное место:
V595 The 'callback_name' pointer was utilized before it was verified against nullptr. Check lines: 5007, 5021. basic_functions.c 5007 Коварная оптимизация V597 The compiler could delete the 'memset' function call, which is used to flush 'final' buffer. The RtlSecureZeroMemory () function should be used to erase the private data. php_crypt_r.c 421 /* * MD5 password encryption. */ char* php_md5_crypt_r (const char *pw, const char *salt, char *out) { static char passwd[MD5_HASH_MAX_LEN], *p; unsigned char final[16]; … /* Don’t leave anything around in vm they could use. */ memset (final, 0, sizeof (final)); //<== return (passwd); } Массив 'final' может содержать приватную информацию о пароле, которая затем обнуляется, но вызов функции 'memset' будет удалён компилятором. Подробности, почему так может произойти и чем это опасно, можно узнать из статьи "Перезаписывать память — зачем?" и описания диагностики V597.Аналогичные места:
V597 The compiler could delete the 'memset' function call, which is used to flush 'final' buffer. The RtlSecureZeroMemory () function should be used to erase the private data. php_crypt_r.c 421 V597 The compiler could delete the 'memset' function call, which is used to flush 'output' buffer. The RtlSecureZeroMemory () function should be used to erase the private data. crypt.c 214 V597 The compiler could delete the 'memset' function call, which is used to flush 'temp_result' buffer. The RtlSecureZeroMemory () function should be used to erase the private data. crypt_sha512.c 622 V597 The compiler could delete the 'memset' function call, which is used to flush 'ctx' object. The RtlSecureZeroMemory () function should be used to erase the private data. crypt_sha512.c 625 V597 The compiler could delete the 'memset' function call, which is used to flush 'alt_ctx' object. The RtlSecureZeroMemory () function should be used to erase the private data. crypt_sha512.c 626 V597 The compiler could delete the 'memset' function call, which is used to flush 'temp_result' buffer. The RtlSecureZeroMemory () function should be used to erase the private data. crypt_sha256.c 574 V597 The compiler could delete the 'memset' function call, which is used to flush 'ctx' object. The RtlSecureZeroMemory () function should be used to erase the private data. crypt_sha256.c 577 V597 The compiler could delete the 'memset' function call, which is used to flush 'alt_ctx' object. The RtlSecureZeroMemory () function should be used to erase the private data. crypt_sha256.c 578 Можем ли мы доверять используемым библиотекам? Стоит отметить, что сторонние библиотеки вносят большой вклад в развитие проекта, позволяя переиспользовать уже реализованные алгоритмы, но за их качеством тоже необходимо следить, как и за основным проектом. Я приведу всего несколько примеров из некоторых библиотек, дабы следовать тематике статьи и просто поднять вопрос о доверии к сторонним библиотекам.В интерпретаторе PHP используется много библиотек, некоторые из них немного переписаны «под себя».
libsqlite
V579 The sqlite3_result_blob function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. sqlite3.c 82631
static void statInit (…)
{
Stat4Accum *p;
…
sqlite3_result_blob (context, p, sizeof (p), stat4Destructor);
…
}
Скорее всего, здесь хотели получить размер объекта, а не указателя. Следовало написать sizeof (*p).pcrelib
V501 There are identical sub-expressions '(1
const pcre_uint32 PRIV (ucp_gbtable[]) = {
(1< (1< Эта статья на английском
Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Svyatoslav Razmyslov. A Post About Analyzing PHP.
