# Заметка про проверку PHP

PHP — скриптовый язык программирования общего назначения, интенсивно применяемый для разработки веб\-приложений\. В настоящее время поддерживается подавляющим большинством хостинг\-провайдеров и является одним из лидеров среди языков программирования, применяющихся для создания динамических веб\-сайтов\.

В случае с компиляторами и интерпретаторами к исходному коду и тестированию, как правило, предъявляются повышенные требования качества и надёжности\. Тем не менее, в исходном коде интерпретатора PHP нашлись подозрительные места\.

В данной статье будут рассмотрены результаты проверки интерпретатора [PHP](https://www.php.net/) [http://www\.asterisk\.org/](https://www.asterisk.org/), полученные с помощью [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/) 5\.18\.

![0277_php_ru/image1.png](https://import.viva64.com/docx/blog/0277_php_ru/image1.png)

## Одинаковые условные выражения

[V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions '\!memcmp\("auto", charset\_hint, 4\)' to the left and to the right of the '\|\|' operator\. html\.c 396

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v605/) Consider verifying the expression: shell\_wrote \> \- 1\. An unsigned value is compared to the number \-1\. php\_cli\.c 266

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v547/) Expression 'tmp\_len \>\= 0' is always true\. Unsigned type value is always \>\= 0\. ftp\_fopen\_wrapper\.c 639

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v555/) The expression 'out\_buf\_size \- ocnt \> 0' will work as 'out\_buf\_size \!\= ocnt'\. filters\.c 1702

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v595/) The 'function\_name' pointer was utilized before it was verified against nullptr\. Check lines: 4859, 4860\. basic\_functions\.c 4859

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/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

```cpp
/*
 * 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' будет удалён компилятором\. Подробности, почему так может произойти и чем это опасно, можно узнать из статьи "[Перезаписывать память \- зачем?](https://pvs-studio.ru/ru/blog/posts/cpp/k0041/)" и описания диагностики [V597](https://pvs-studio.ru/ru/docs/warnings/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](https://pvs-studio.ru/ru/docs/warnings/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

```cpp
static void statInit(....)
{
  Stat4Accum *p;
  ....
  sqlite3_result_blob(context, p, sizeof(p), stat4Destructor);
  ....
}
```

Скорее всего, здесь хотели получить размер объекта, а не указателя\. Следовало написать sizeof\(\*p\)\.

### pcrelib

[V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions '\(1 << ucp\_gbL\)' to the left and to the right of the '\|' operator\. pcre\_tables\.c 161

```cpp
const pcre_uint32 PRIV(ucp_gbtable[]) = {
  (1<<ucp_gbLF),
  0,
  0,
  ....
  (1<<ucp_gbExtend)|(1<<ucp_gbSpacingMark)|(1<<ucp_gbL)|    // <=
    (1<<ucp_gbL)|(1<<ucp_gbV)|(1<<ucp_gbLV)|(1<<ucp_gbLVT), // <=

   (1<<ucp_gbExtend)|(1<<ucp_gbSpacingMark)|(1<<ucp_gbV)|
     (1<<ucp_gbT),
  ....
};
```

В выражении для вычисления одного элемента массива есть повторяющееся \(1<<ucp\_gbL\)\. Судя по коду далее, одна из переменных ucp\_gbL вполне могла бы называться ucp\_gbT, либо просто лишняя\.

### PDO

[V595](https://pvs-studio.ru/ru/docs/warnings/v595/) The 'dbh' pointer was utilized before it was verified against nullptr\. Check lines: 103, 110\. pdo\_dbh\.c 103

```cpp
PDO_API void pdo_handle_error(pdo_dbh_t *dbh, ....)
{
  pdo_error_type *pdo_err = &dbh->error_code;  // <=
  ....
  if (dbh == NULL || dbh->error_mode == PDO_ERRMODE_SILENT) {
    return;
  }
  ....
}
```

Здесь в самом начале функции выполняется разыменование пришедшего указателя, а далее он проверяется на валидность\.

### libmagic

[V519](https://pvs-studio.ru/ru/docs/warnings/v519/) The '\* code' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 100, 101\. encoding\.c 101

```cpp
protected int file_encoding(...., const char **code, ....)
{
  if (looks_ascii(buf, nbytes, *ubuf, ulen)) {
    ....
  } else if (looks_utf8_with_BOM(buf, nbytes, *ubuf, ulen) > 0) {
    DPRINTF(("utf8/bom %" SIZE_T_FORMAT "u\n", *ulen));
    *code = "UTF-8 Unicode (with BOM)";
    *code_mime = "utf-8";
  } else if (file_looks_utf8(buf, nbytes, *ubuf, ulen) > 1) {
    DPRINTF(("utf8 %" SIZE_T_FORMAT "u\n", *ulen));
    *code = "UTF-8 Unicode (with BOM)";                     // <=
    *code = "UTF-8 Unicode";                                // <=
    *code_mime = "utf-8";
  } else if (....) {
    ....
  }
}
```

Два раза задали кодировку в переменную, одна строчка лишняя и, возможно, приводит к неправильному поведению программы далее\.

## Заключение

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

Используя статический анализ регулярно, можно сэкономить массу времени на решение более полезных задач\.