Знову перевіряємо Apache HTTP Server

Проект Apache HTTP Server продовжує розвиватись. Аналізатор PVS-Studio не відстає і стає все більш потужним із кожною новою версією. Подивимося, які результати покаже перевірка цього разу.

Apache HTTP Server - це кроссплатформенний проект з відкритим вихідним кодом. Проект складається з багатьох модулів. Ядро HTTP Server повністю розроблене командою Apache Software Foundation мовою С. Інші компоненти сервера створені різними сторонніми розробниками, які підтримують ініціативу відкритого програмного забезпечення.

Ранні версії Apache HTTP Server перевірялися за допомогою Сoverity. У перевіреній версії було виявлено слідів перевірки іншими аналізаторами. Якість коду проекту на найвищому рівні. Незважаючи на це аналізатор PVS-Studio зміг виявити кілька цікавих помилок.

Проект уже перевірявся у 2011 році. Про знайдені на той час помилки можна прочитати у статті "Лев Толстой і статичний аналіз коду".

Для перевірки використовувався аналізатор PVS-Studio версії 6.08.

Неправильна перевірка, що рядок порожній

V528 It is odd that pointer to 'char' typ is compared with the '\0' value. Probably meant: ** ctx->re_source == '\0'. util_expr_eval.c 199

Працюючи з покажчиками іноді виникає плутанина між покажчиком і значеннями, куди він вказує. У прикладі видно, що під час перевірки третього подвыражения за умови забули розіменувати покажчик. Розробник припускає, що він перевіряє, чи є рядок порожнім чи ні. Для цього потрібно порівняти перший символ рядка із термінальним нулем. Але насправді відбувається порівняння покажчика із нульовим символом. Після виправлення виразу стає очевидним, що код також слід доповнити перевіркою на існування покажчика нарядок.

Аналізатор вже виявляв цю помилку в проекті, про що свідчить запис на сторінці з прикладами помилок, які є діагностикою V528. Оскільки помилка, як і раніше, присутня в проекті, про неї слід згадати знову. Для виправлення потрібно змінити код таким чином:

Інкрементування вказівника замість значення

V532 Consider inspecting thetetement of 'pointer++' pattern. Probably meant: '(*pointer)++'. iconv_uc.c 114

У коді використовується операція розіменування покажчика. Отримане значення у своїй не використовується. Судячи з коду функції, працювати потрібно саме зі значенням покажчика. Отже, для виправлення помилки треба підвищити пріоритет операції розіменування за допомогою дужок: (*res)++;

Неправильне затирання пароля

V597 Комп'ютер може вимкнути 'memset' функцію call, яка використовується для flush 'buf' buffer. RtlSecureZeroMemory() функція повинна бути використана для private data. passwd_common.c 165

Обов'язок будь-якої програми, що працює з конфіденційними даними, затирати в пам'яті паролі та іншу важливу інформацію після завершення роботи з нею. У цьому фрагменті розробники намагаються обнулити буфер, який використовувався для зберігання пароля. Вибраний ними спосіб мав виконати це завдання. Однак для того, щоб функція memset виконала завдання правильно, буфер повинен використовуватись у наступному коді. Якщо пам'ять заповнюється нулями, а потім не використовується, компілятор має право видалити функцію memset при побудові коду. В результаті інформація, яка має бути знищена, залишається в пам'яті. Невідомо, що буде з цим регіоном пам'яті після і куди потрапить ця незнищена інформація. Для очищення пам'яті необхідно використовувати спеціальні функції, такі якRtlSecureZeroMemory() або memset_s() .

Хочеться відзначити, що це, мабуть, найсерйозніші зі знайдених дефектів для програми Apache HTTP Server!

Ще кілька повідомлень знайдених цією діагностикою:

V597 Комп'ютер може вимкнути 'memset' функцію call, яка використовується для flush 'x' buffer. RtlSecureZeroMemory() функція повинна бути використана для private data. apr_md4.c 362
V597 Комп'ютер може вимкнути 'memset' функцію call, яка використовується для flush 'tmpbuf' buffer. RtlSecureZeroMemory() функція повинна бути використана для private data. apr_md5.c 436
V597 Комп'ютер може вимкнути 'memset' функцію call, яка використовується для flush 'final' buffer. RtlSecureZeroMemory() функція повинна бути використана для private data. apr_md5.c 662

Неініціалізована змінна

V614 Потенційно uninitalizated pointer 'wch' used. start.c 58

Функція готує інформацію для конвертування рядка з W >args містить негативне значення, отже кількість символів у рядку невідома, і їх потрібно порахувати.

На даний момент функція використовується в проекті лише один раз зі значенням args рівним -1. Тому ця помилка довгий час залишалася б непомітною, доки не знадобилося використовувати функцію з позитивним значенням args. Я не можу знати, що планувалося робити всередині цієї функції у такій ситуації. Як мінімум дивно бачити у вигляді параметра значення, що в результаті повертається як результат її роботи. У той час як присутність умовного оператора на початку функції, у разі позитивного значення args, робить її виконання абсолютно безглуздим.

Підозральний вираз

V694 Condition ((s + 1) != ((void *) 0)) isтільки false if there is pointer overflow which is undefined behaviour anyway. mod_mime.c 531

Дуже підозріла умова. Перше вираження за умови може бути хибним лише тому випадку, якщо відбудеться переповнення при складанні покажчика з одиницею. Якщо відбувається переповнення покажчика, виникає невизначена поведінка, отже цей код у разі некоректний.

Неправильна перевірка HRESULT

V545 Такий конкретний вираз 'if' operator є неправильним для типу HRESULT типу 'SHGetMalloc(& p; pMalloc)'. The SUCCEEDED or FAILED macro should be used instead. apachemonitor.c 915

SHGetMalloc є системною функцією і повертає результат типу HRESULT . HRESULT 32-розрядне значення, що логічно складається з трьох полів. Його не можна використовувати як значення типу bool. Для обробки таких значень необхідно використовувати макрос SUCCEEDED.

Зайва операція?

V560 Apart of conditional expression is always true: (rv == 0). config.c 2029

Аналізатор вбачав зайву перевірку усередині умови. На перший погляд, це просто надлишковий код. При докладному розгляді видно, що таке значення змінної rv не дозволило б дійти входу в цикл. До того ж дивно, навіщо використовувати значення змінної, що залишилося після виконання попередніх операцій, якщо в тілі циклу вона більше ніде не використовується.

Логічно припустити, що перед умовою треба було використовувати функцію rv = apr_dir_open(. ); і тоді перевірка результату rv набуває сенсу. Звичайно, можу помилятися і це просто зайва перевірка, але, на мій погляд, фрагмент коду обов'язково треба перевірити. Можливо, розробники побачать, що в код циклу та операцій, що в ньому обробляються, дійсно закралася помилка і зможуть вчасно її виправити.

Ще кілька схожих помилок:

V560 Apart of conditional expression is always true: status == 0. mod_ & Попередження:

V571 Recurring check. The 'ldc->ChaseReferrals == 1' condition was already verified in line 399. util_ldap.c 400

У цьому прикладі наведено надмірну умову. Немає необхідності перевіряти одне й теж вираз у внутрішньому та зовнішньому умовному операторі, оскільки умовою входу у внутрішній оператор є проходження перевірок зовнішнього. В даному випадку весь код, закладений всередині операторів, вимагає перевірки всіх виразів в обох операторах if . Тому логічніше прибрати зовнішній умовний оператор. А для збереження послідовності перевірок пропоную трохи змінити вираз внутрішнього умовного оператора.

Неправильна директива

V665 Можливо, використання '#pragma warning(default: X)' is incorrect in this context. The '#pragma warning(push/pop)' should be used instead. Check lines: 38, 40. apr_getpass.c 40

У наведеному фрагменті коду розробники замість повернення директиві попереднього значення використовують значення за промовчанням для директиви. Це хибний підхід. У разі необхідно діяти інакше: запам'ятати попереднє значення з допомогою директиви #pragma warning(push) , і повернути його #pragma warning(pop) . Виправлений код:

Висновок

Знайдені недоліки показують, що помилки можуть залишитись навіть у найякісніших і добре протестованих проектах. Статичний аналіз потребує регулярного підходу. Одноразової перевірки недостатньо. Як би не був досвідчений програміст, він все одно може допускати помилки та інші помилки. Аналізатор PVS-Studio дозволить виявити помилки та підозрілі місця раніше, ніж вони позначаться на підсумковому релізі. Пропоную скачатиспробувати аналізатор PVS-Studio самостійно.

Знайдіть помилки у своєму C, C++, C# та Java коді

Пропонуємо перевірити код вашого проекту за допомогою аналізатора коду PVS-Studio. Одна знайдена у ньому помилка скаже вам про користь методології статичного аналізу коду більше десятка статей.