• Партнер

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

    Зачем нужен статический анализатор

    Сов­ремен­ные при­ложе­ния — это огромные мас­сивы кода и биб­лиотек. Усле­дить за ошиб­ками в них — дело неп­ростое. Мож­но забыть выс­вободить ресур­сы, сде­лать «удач­ную» опе­чат­ку, при которой код собира­ется, но ведет себя неп­равиль­но, допус­тить утеч­ку памяти... Короче, есть мно­жес­тво ситу­аций, которые спо­соб­ны прев­ратить отладку в изма­тыва­ющее прик­лючение. И если ты ока­зал­ся в одной из них, это вов­се не зна­чит, что ты пло­хой прог­раммист. Прос­то люди могут уста­вать, отвле­кать­ся и допус­кать ошиб­ки.

    Для помощи прог­раммис­там при­дума­ли спе­циаль­ные тул­зы — ста­тичес­кие ана­лиза­торы кода, которые помога­ют в раз­работ­ке и выяв­ляют раз­ные ошиб­ки: логичес­кие баги, опе­чат­ки, опас­ные конс­трук­ции, несо­ответс­твия каким‑то стан­дартам и кон­венци­ям и так далее. Ста­тичес­кие ана­лиза­торы при­меня­ются имен­но на эта­пе раз­работ­ки, а не тес­тирова­ния, поэто­му поз­воля­ют пра­вить код на мес­те, что нам­ного про­ще и быс­трее.

    Да­вай пос­мотрим, как обра­щать­ся с ана­лиза­тором на при­мере PVS-Studio. Я покажу, какие ошиб­ки будут им ловить­ся, а за чем при­дет­ся сле­дить самос­тоятель­но. PVS-Studio «понима­ет» C/C++, C# и Java, под­держи­вает Windows, Linux и macOS. Но пос­коль­ку показы­вать я буду всё на рабочем про­екте, то сфо­куси­руюсь на C++ и Windows.

     

    Установка и настройка

    Итак, кача­ем PVS-Studio с сай­та раз­работ­чика. На момент написа­ния статьи акту­аль­ная вер­сия — 7.19.

    Промо

    Что­бы получить бес­плат­ную проб­ную лицен­зию на 30 дней, перей­ди по ссыл­ке. В поле «про­мокод» будет встав­лен код xakep, который ком­пания‑раз­работ­чик пре­дос­тавила для читате­лей «Хакера».

    Что инте­рес­но, инстал­лятор сра­зу нашел у меня на машине Visual Studio 2022 и IntelliJ IDEA Ultimate 2022 и сра­зу пред­ложил интегри­ровать­ся в них. Не отка­зыва­емся, потому что будет удоб­но исполь­зовать ста­тана­лиза­тор не как отдель­ную тул­зу, а имен­но как пла­гин для IDE.

    Инсталятор PVS-Studio детектит установленные IDE
    Ин­ста­лятор PVS-Studio детек­тит уста­нов­ленные IDE

    Те­перь откро­ем Visual Studio и прой­дем­ся по самым инте­рес­ным нас­трой­кам.

    Настройки PVS-Studio
    Нас­трой­ки PVS-Studio

    Об­рати вни­мание на IntermoduladAnalysisCpp, что озна­чает «меж­модуль­ный ана­лиз». Эта нас­трой­ка при­меня­ется толь­ко к про­ектам на C++. Раз­работ­чики уве­ряют, что при вклю­чении этой опции ана­лиза­тор про­изво­дит ана­лиз более эффектив­но. Про­верим! Вот два ска­ниро­вания: без этой опции и вмес­те с ней.

    Без межмодульного анализа
    Без меж­модуль­ного ана­лиза
    С включенным межмодульным анализом
    С вклю­чен­ным меж­модуль­ным ана­лизом

    Как видишь, ста­ло боль­ше на одну ошиб­ку клас­са Low. Не очень впе­чат­ляет, но галоч­ку луч­ше оста­вить.

    Сле­дующая инте­рес­ная опция — No Noise. Она исклю­чает низ­копри­ори­тет­ные пре­дуп­режде­ния, что­бы было про­ще ори­енти­ровать­ся в выдаче ана­лиза­тора.

    Ре­комен­дую заг­лянуть и во вклад­ку Detectable Errors — здесь мож­но вклю­чать или вык­лючать обна­руже­ние опре­делен­ных групп оши­бок.

    PVS-Studio уме­ет опре­делять потен­циаль­ные уяз­вимос­ти, свя­зан­ные с безопас­ностью кода. Сре­ди нацелен­ных на это наборов пра­вил:

    • CWE — база конс­трук­ций язы­ка, которые содер­жат потен­циаль­ные уяз­вимос­ти;
    • AUTOSAR — набор пра­вил для стан­дарта C++ 14 для встра­иваемых отка­зоус­той­чивых сис­тем;
    • MISRA — пра­вила, вырабо­тан­ные ком­пани­ей MISRA для встра­иваемых сис­тем, где важ­ны безопас­ность и отка­зоус­той­чивость;
    • OWASP — пра­вила для безопас­ной раз­работ­ки веб‑при­ложе­ний;
    • SEI-CERT — пра­вила, соз­данные цен­тром CERT, опять же, для повыше­ния надеж­ности ПО.

    Ре­комен­дую вклю­чить все эти под­сказ­ки и обра­щать на них вни­мание при написа­нии кода. Они зна­читель­но уве­личат шанс не про­пус­тить потен­циаль­ный баг.

    По­мимо это­го, PVS-Studio опре­деля­ет проб­лемы 64-бит­ного кода, пред­лага­ет опти­миза­ции про­изво­дитель­нос­ти и даже содер­жит нес­коль­ко пра­вил (Custom), которые сде­ланы спе­циаль­но по заяв­кам тру­дящих­ся.

     

    Анализатор на практике: разбираем примеры

    Те­перь перей­дем к обзо­ру най­ден­ных при помощи PVS-Studio оши­бок и недоче­тов. Весь код, который будет тут пред­став­лен для при­мера, — реаль­ный, из круп­ного опен­сор­сно­го про­екта, который раз­вива­ется нес­коль­ко лет. «Син­тетичес­ких» оши­бок, искусс­твен­но соз­данных для демонс­тра­ции воз­можнос­тей ста­тана­лиза­тора, я при­водить не буду, ведь это не так инте­рес­но.

    За­пус­каем ска­ниро­вание! Пер­вый недочет, най­ден­ный ана­лиза­тором и име­ющий рей­тинг Medium:

    V560 A part of conditional expression is always true: threadArg

    Ес­ли нажать на номер ошиб­ки, откро­ется сайт раз­работ­чика с пояс­нени­ем (как на рус­ском, так и на англий­ском язы­ках): «Ана­лиза­тор обна­ружил потен­циаль­но воз­можную ошиб­ку внут­ри логичес­кого усло­вия. Часть логичес­кого выраже­ния всег­да истинно и оце­нено как опас­ное». Раз­берем­ся...

    EncryptionThreadPool.c 205:
    static TC_THREAD_PROC EncryptionThreadProc (void *threadArg)
    {
    EncryptionThreadPoolWorkItem *workItem;
    if (threadArg)
    {
    #ifdef DEVICE_DRIVER
    SetThreadCpuGroupAffinity ((USHORT) *(WORD*)(threadArg));
    #else
    SetThreadGroupAffinityFn SetThreadGroupAffinityPtr = (SetThreadGroupAffinityFn) GetProcAddress (GetModuleHandle (L"kernel32.dll"), "SetThreadGroupAffinity");
    if (SetThreadGroupAffinityPtr && threadArg)
    {
    GROUP_AFFINITY groupAffinity = {0};
    groupAffinity.Mask = ~0ULL;
    groupAffinity.Group = *(WORD*)(threadArg);
    SetThreadGroupAffinityPtr(GetCurrentThread(), &groupAffinity, NULL);
    }
    #endif
    }

    Ана­лиза­тор руга­ется вот на эту стро­ку:

    if (SetThreadGroupAffinityPtr && threadArg)

    Прос­мотрим начало всей фун­кции: если ука­затель threadArg валиден, мы вхо­дим в тело if. Далее, в нем есть еще один вло­жен­ный if с усло­вием SetThreadGroupAffinityPtr && threadArg, на который как раз и жалу­ется ана­лиза­тор. Но threadArg мы ведь уже про­вери­ли, нет нуж­ды его про­верять опять! Код избы­точ­ный, хоть и не опас­ный. Сме­ло уби­раем часть усло­вия && threadArg и идем даль­ше.

    Ана­лиза­тор жалу­ется вот на этот код:

    ci = crypto_open ();
    if (!ci)
    return FALSE;
    if (QueryPerformanceFrequency (&benchmarkPerformanceFrequency) == 0)
    {
    if (ci)
    crypto_close (ci);
    MessageBoxW (hwndDlg, GetString ("ERR_PERF_COUNTER"), lpszTitle, ICON_HAND);
    return FALSE;
    }

    Ошиб­ка:

    V547. Expression is always true/false

    «Усло­вие всег­да истинно или лож­но». В этом при­мере про­вер­ка сущес­тво­вания ci уже есть:

    if (!ci)
    return FALSE;

    Да­лее идет еще одна, хотя никаких манипу­ляций с ci нет:

    if (ci)
    crypto_close (ci);

    Опять же нек­ритич­но, но код мож­но почис­тить и пох­валить ана­лиза­тор за помощь. Едем даль­ше, наш­ли сра­зу три недос­татка в такой фун­кции:

    void DisableCPUExtendedFeatures ()
    {
    g_hasSSE2 = 0;
    g_hasISSE = 0;
    g_hasMMX = 0;
    g_hasSSE2 = 0;
    g_hasISSE = 0;
    g_hasMMX = 0;
    g_hasAVX = 0;
    g_hasAVX2 = 0;
    g_hasBMI2 = 0;
    g_hasSSE42 = 0;
    g_hasSSE41 = 0;
    g_hasSSSE3 = 0;
    g_hasAESNI = 0;
    g_hasCLMUL = 0;
    }

    Ошиб­ка:

    V1048. Variable 'g_hasSSE2' was assigned the same value

    Под­сказ­ка с сай­та: «Ана­лиза­тор обна­ружил прис­ваива­ние перемен­ной зна­чения, которое уже содер­жится в ней. Ско­рее все­го, это логичес­кая ошиб­ка». При этом ошиб­ки оди­нако­вые для g_hasSSE2, g_hasISSE, g_hasMMX.

    Смот­рим вни­матель­нее и понима­ем, что без­болез­ненно можем уда­лить вот эти стро­ки:

    g_hasSSE2 = 0;
    g_hasISSE = 0;
    g_hasMMX = 0;

    Даль­ше вот такое инте­рес­ное пре­дуп­режде­ние:

    V1042 This file is marked with copyleft license, which requires you to open the derived source code. jitterentropy.h 23

    Оно сра­бота­ло на стан­дар­тный текст лицен­зии GNU. Ана­лиза­тор пояс­няет: «Ана­лиза­тор обна­ружил в фай­ле copyleft лицен­зию, которая обя­зыва­ет открыть осталь­ной исходный код. Это может быть неп­рием­лемо для мно­гих ком­мерчес­ких про­ектов». С одной сто­роны, мы тут вро­де бы ана­лизи­руем код, а не юри­дичес­кие тон­кости. С дру­гой — в мире еще есть раз­работ­чики, которые не зна­ют о копилеф­те и дума­ют, что раз код откры­тый, зна­чит мож­но брать. Опять же, если ошиб­ка не полез­на, ты можешь ее отклю­чить.

    А вот инте­рес­но сра­бота­ли пра­вила OWASP (прав­да на тес­товом коде про­екта):

    derive_key_streebog ("password", 8, "\x12\x34\x56\x78", 4, 5, dk, 96);

    Ошиб­ка:

    V5013 [OWASP-2.10.4] Storing credentials inside source code can lead to security issues. Tests.c 1762

    По­ясне­ние: «Ана­лиза­тор обна­ружил в коде дан­ные, которые могут являть­ся кон­фиден­циаль­ными». Ну, тут и добавить нечего. Ана­лиза­тор заподоз­рил, что в коде пря­мым тек­стом ука­заны пароли, и ока­зал­ся прав! Но в дан­ном слу­чае это механизм тес­тирова­ния, и пароль нек­ритич­ный.

    Те­перь вклю­чим ана­лиз 64-бит­ного кода. Ана­лиза­тор сра­зу зап­риметил вот такую строч­ку:

    GlobalMemoryStatus (&memoryStatus);

    Ошиб­ка:

    V303 The function 'GlobalMemoryStatus' is deprecated in the Win64 system. It is safer to use the 'GlobalMemoryStatusEx' function. Random.c 875

    Дру­гими сло­вами, на 64-бит­ной плат­форме фун­кцию GlobalMemoryStatus при­менять не рекомен­дует­ся и ее луч­ше заменить на GlobalMemoryStatusEx. Впро­чем, о той же ошиб­ке сооб­щает и сам Visual Studio.

    У ана­лиза­тора могут быть и лож­ные сра­баты­вания. К при­меру, получа­ем вот такую ошиб­ку уров­ня High:

    V547 Expression is always true. Crypto.c 1465)

    По­доз­рение выз­вала вот эта стро­ка:

    VcProtectMemory (encID, pCryptoInfo->ks, MAX_EXPANDED_KEY,

    И сле­дующая:

    pCryptoInfo->ks2, MAX_EXPANDED_KEY,

    Взгля­нем на всю фун­кцию.

    static void VcInternalProtectKeys (PCRYPTO_INFO pCryptoInfo, uint64 encID)
    {
    #ifdef TC_WINDOWS_DRIVER
    VcProtectMemory (encID, pCryptoInfo->ks, MAX_EXPANDED_KEY, pCryptoInfo->ks2, MAX_EXPANDED_KEY);
    #else
    VcProtectMemory (encID, pCryptoInfo->ks, MAX_EXPANDED_KEY,
    pCryptoInfo->ks2, MAX_EXPANDED_KEY,
    pCryptoInfo->master_keydata, MASTER_KEYDATA_SIZE,
    pCryptoInfo->k2, MASTER_KEYDATA_SIZE);
    #endif
    }

    Да­лее пос­мотрим на про­тотип VcProtectMemory:

    void VcProtectMemory (uint64 encID, unsigned char* pbData, size_t cbData,
    unsigned char* pbData2, size_t cbData2,
    unsigned char* pbData3, size_t cbData3,
    unsigned char* pbData4, size_t cbData4)

    Оче­вид­но, это ошиб­ка ана­лиза­тора (точ­нее, син­такси­чес­кого пар­сера, как я понимаю), потому что никаких усло­вий здесь нет и всег­да истинны­ми они быть не могут. Видимо, при­чина такого поведе­ния — круп­ный мак­рос, который переда­ется в качес­тве аргу­мен­та.

    Или вот инте­рес­ное поведе­ние. Воп­росы выз­вал такой код.

    BOOL bNewDesktopSet = FALSE;
    // wait for SwitchDesktop to succeed before using it for current thread
    while (true)
    {
    if (SwitchDesktop (pParam->hDesk))
    {
    bNewDesktopSet = TRUE;
    break;
    }
    Sleep (SECUREDESKTOP_MONOTIR_PERIOD);
    }
    if (bNewDesktopSet)
    {
    SetThreadDesktop (pParam->hDesk);

    Ана­лиза­тор руга­ется на стро­ку if (bNewDesktopSet) со сле­дующим вер­диктом:

    V547 Expression 'bNewDesktopSet' is always true. Dlgcode.c 14113

    Да­вай раз­бирать­ся: bNewDesktopSet ини­циали­зиру­ется как FALSE при объ­явле­нии, далее вхо­дим в цикл, в котором перек­лючение bNewDesktopSet на TRUE воз­можно толь­ко в том слу­чае, если сра­бота­ет WinAPI SwitchDesktop. Де‑юре ана­лиза­тор прав, но прав ли он по сути? Во‑пер­вых, мы не можем быть уве­рены, про­изой­дет ли событие SwitchDesktop (pParam->hDesk), потому что за поведе­ние WinAPI мы не отве­чаем. Во‑вто­рых, взгля­ни на архи­тек­туру кода: выпол­нение тела if отда­но на откуп поведе­нию фун­кции WinAPI SwitchDesktop, которая или сра­бота­ет (будет переход), или обра­зует веч­ный цикл, потому как while (true). На мой взгляд, ошиб­ки «Expression ... is always true» в таком слу­чае быть не дол­жно.

    Даль­ше встре­чаем такую ошиб­ку:

    V112 Dangerous magic number

    А вот стро­ка‑наруши­тель:

    unsigned __int32 CRC = 0xffffffff;

    Ошиб­ка озна­чает, что исполь­зует­ся «магичес­кая» кон­стан­та, о пред­назна­чении который зна­ет толь­ко тот, кто ее написал.

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

     

    Выводы

    Про­верив наш про­ект, ана­лиза­тор сде­лал нес­коль­ко сотен замеча­ний — и это толь­ко по катего­рии General, без вклю­чения допол­нитель­ных про­верок, ина­че счет пошел бы на тысячи! Раз­бирать их все в статье нет смыс­ла, так она прев­ратит­ся в мно­гос­тра­нич­ный отчет по раз­бору выдачи PVS-Studio — со ссыл­ками на докумен­тацию и выдер­жка­ми из Стра­устру­па.

    Воз­можно, у тебя воз­никнет воп­рос о том, зачем вооб­ще нуж­ны сто­рон­ние ста­тичес­кие ана­лиза­торы, если во мно­гие IDE они и так встро­ены. Все дело в количес­тве и качес­тве про­верок. Ког­да ана­лиза­тор — это основной про­ект для раз­работ­чика, то мож­но ожи­дать, что ана­лиз будет более тща­тель­ным и глу­боким (при­мер­но как Windows Defender не заменит нас­тоящий прод­винутый фай­рвол). Так и ока­залось в слу­чае с PVS-Studio.

    Не думай при этом, что ста­тичес­кий ана­лиза­тор — это вол­шебная палоч­ка, которая выручит тебя из любой беды и будет писать хороший код за тебя. Работа с ана­лиза­тором — это преж­де все­го раз­гре­бание отче­тов и вдум­чивое чте­ние кода. Авто­мати­чес­кий инс­тру­мент тоже будет оши­бать­ся, прос­то не так же, как человек. Зато вмес­те вы дос­тигне­те боль­шего! Для раз­работ­чиков боль­ших про­ектов это однознач­но мас­тхэв.

    Подписаться
    Уведомить о
    1 Комментарий
    Старые
    Новые Популярные
    Межтекстовые Отзывы
    Посмотреть все комментарии