Замітка про перевірку 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 & gt; — 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 & gt; = 0' is always true. Unsigned type value is always & gt; = 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 & gt; = 0) є некоректною. В гіршому випадку декремент може привести до переповнення індексу та обігу до пам'яті за кордоном масиву. Швидше за все, код виконується вірно завдяки додатковим умовам і коректним вихідними даними, але небезпека «зациклення» або виходу за межі масиву тут присутня.
 
 Різниця беззнакових чисел
 V555 The expression 'out_buf_size — ocnt & gt; 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' буде видалено компілятором. Подробиці, чому так може статися і чим це небезпечно, можна дізнатися зі статті & quot; Перезаписувати пам'ять — навіщо? & quot; і описи діагностики 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 & lt; & lt; ucp_gbL)' to the left and to the right of the '|' operator. pcre_tables.c 161
 
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 & lt; & lt; ucp_gbL). Судячи по коду далі, одна з змінних ucp_gbL цілком могла б називатися ucp_gbT, або просто зайва.
 
 PDO
 V595 The 'dbh' pointer was utilized before it was verified against nullptr. Check lines: 103, 110. pdo_dbh.c 103
 
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 The '* code' variable is assigned values ​​twice successively. Perhaps this is a mistake. Check lines: 100, 101. encoding.c 101
 
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 існує давно і популярний, в його коді і використовуваних бібліотеках все ж знайшлися підозрілі місця, хоча подібного роду проект напевно перевіряється різними аналізаторами.
 
Використовуючи статичний аналіз регулярно, можна заощадити масу часу на вирішення більш корисних завдань.
 
 цю статтю англійською
Якщо хочете поділитися цією статтею з англомовною аудиторією, то прошу використати посилання на переклад: Svyatoslav Razmyslov. A Post About Analyzing PHP .
 
 Прочитали статтю і є питання? Часто до наших статей ставлять одні і ті ж питання. Відповіді на них ми зібрали тут: Відповіді на питання читачів статей про PVS-Studio і CppCat, версія 2014 . Будь ласка, ознайомтеся зі списком.
 
    
Джерело: Хабрахабр

0 коментарів

Тільки зареєстровані та авторизовані користувачі можуть залишати коментарі.