Мы используем куки, чтобы пользоваться сайтом было удобно.
Хорошо
to the top
close form

Заполните форму в два простых шага ниже:

Ваши контактные данные:

Шаг 1
Поздравляем! У вас есть промокод!

Тип желаемой лицензии:

Шаг 2
Team license
Enterprise license
** Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности
close form
Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
RUB
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
Бесплатная лицензия PVS‑Studio для специалистов Microsoft MVP
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
Мне интересно попробовать плагин на:
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
check circle
Ваше сообщение отправлено.

Мы ответим вам на


Если вы так и не получили ответ, пожалуйста, проверьте папку
Spam/Junk и нажмите на письме кнопку "Не спам".
Так Вы не пропустите ответы от нашей команды.

Вебинар: ГОСТ Р 71207–2024. Статический анализ программного обеспечения. Общее описание и актуальность - 15.07

>
>
>
Конкурс внимательности: PVS-Studio vs Х…

Конкурс внимательности: PVS-Studio vs Хакер

21 Июл 2022

Время от времени мы пишем статьи в духе "статический анализатор внимательнее C++ программиста". Сегодня мы продолжим эту традицию, разве что заменив "программист" на "хакер".

0970_hacker_comment_ru/image1.png

Про наш статический анализатор написали небольшую обзорную статью в журнале Хакер: "PVS-Studio. Тестируем статический анализатор кода на реальном проекте". Моё внимание привлёк разбор следующего фрагмента кода:

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" в таком случае быть не должно.

Автор публикации поспешил классифицировать сообщение анализатора как ложноположительное срабатывание, не разобравшись в коде. Давайте ещё раз внимательно взглянем на вечный цикл:

while (true)
{
  if (SwitchDesktop (pParam->hDesk))
  {
    bNewDesktopSet = TRUE;
    break;
  }
  Sleep (SECUREDESKTOP_MONOTIR_PERIOD);
}

if (bNewDesktopSet)  // <= V547

Код ниже цикла может начать выполняться в одном единственном случае – сработает оператор break. Обратите внимание, что вызов оператора break всегда сопровождается присваиванием переменной bNewDesktopSet значения TRUE.

Поэтому если цикл прекратил своё выполнение, то переменная bNewDesktopSet однозначно будет равна TRUE. Анализатор это понимает, основываясь на анализе потока данных (см. "Технологии статического анализа кода PVS-Studio").

Автор рассуждает о том, сработает или нет условие SwitchDesktop(pParam->hDesk). Но эти рассуждения не имеют значения. Если не сработает – цикл не закончится. Если сработает – то выполнится присваивание bNewDesktopSet = TRUE. Поэтому анализатор абсолютно прав, выдавая предупреждение.

Анализатором найдена настоящая ошибка или просто избыточный код?

Для этого обратимся к первоисточнику. В статье не говорится, какой проект анализировался, но, немного погуглив, легко понять, что это VeraCrypt. Вот функция, содержащая рассмотренный нами фрагмент кода:

static DWORD WINAPI SecureDesktopThread(LPVOID lpThreadParameter)
{
  volatile BOOL bStopMonitoring = FALSE;
  HANDLE hMonitoringThread = NULL;
  unsigned int monitoringThreadID = 0;
  SecureDesktopThreadParam* pParam =
    (SecureDesktopThreadParam*) lpThreadParameter;
  SecureDesktopMonitoringThreadParam monitorParam;
  HDESK hOriginalDesk = GetThreadDesktop (GetCurrentThreadId ());
  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);

    // create the thread that will ensure that VeraCrypt secure desktop
    // has always user input
    monitorParam.szVCDesktopName = pParam->szDesktopName;
    monitorParam.hVcDesktop = pParam->hDesk;
    monitorParam.pbStopMonitoring = &bStopMonitoring;
    hMonitoringThread =
      (HANDLE) _beginthreadex (NULL, 0, SecureDesktopMonitoringThread,
                               (LPVOID) &monitorParam, 0, &monitoringThreadID);
  }

  pParam->retValue = DialogBoxParamW (pParam->hInstance, pParam->lpTemplateName,
            NULL, pParam->lpDialogFunc, pParam->dwInitParam);

  if (hMonitoringThread)
  {
    bStopMonitoring = TRUE;

    WaitForSingleObject (hMonitoringThread, INFINITE);
    CloseHandle (hMonitoringThread);
  }

  if (bNewDesktopSet)
  {
    SetThreadDesktop (hOriginalDesk);
    SwitchDesktop (hOriginalDesk);
  }

  return 0;
}

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

Возможно, цикл должен быть невечным и прерываться по истечению какого-то времени. Тогда перед нами недописанный некорректный код. Т.е. перед нами задумка, которую не довели до конца.

Другой вариант – код подвергался постепенным правкам и в какой-то момент стал избыточным, но этого не заметили. В этом случае функцию можно упростить, удалив часть бессмысленных проверок:

static DWORD WINAPI SecureDesktopThread(LPVOID lpThreadParameter)
{
  volatile BOOL bStopMonitoring = FALSE;
  HANDLE hMonitoringThread = NULL;
  unsigned int monitoringThreadID = 0;
  SecureDesktopThreadParam* pParam =
    (SecureDesktopThreadParam*) lpThreadParameter;
  SecureDesktopMonitoringThreadParam monitorParam;
  HDESK hOriginalDesk = GetThreadDesktop (GetCurrentThreadId ());

  // wait for SwitchDesktop to succeed before using it for current thread
  while (!SwitchDesktop (pParam->hDesk))
  {
    Sleep (SECUREDESKTOP_MONOTIR_PERIOD);
  }

  SetThreadDesktop (pParam->hDesk);

  // create the thread that will ensure that VeraCrypt secure desktop
  // has always user input
  monitorParam.szVCDesktopName = pParam->szDesktopName;
  monitorParam.hVcDesktop = pParam->hDesk;
  monitorParam.pbStopMonitoring = &bStopMonitoring;
  hMonitoringThread =
    (HANDLE) _beginthreadex (NULL, 0, SecureDesktopMonitoringThread,
                             (LPVOID) &monitorParam, 0, &monitoringThreadID);

  pParam->retValue = DialogBoxParamW (pParam->hInstance, pParam->lpTemplateName,
            NULL, pParam->lpDialogFunc, pParam->dwInitParam);

  if (hMonitoringThread)
  {
    bStopMonitoring = TRUE;

    WaitForSingleObject (hMonitoringThread, INFINITE);
    CloseHandle (hMonitoringThread);
  }

  SetThreadDesktop (hOriginalDesk);
  SwitchDesktop (hOriginalDesk);

  return 0;
}

Возможно, чуть нагляднее изменения будут видны на diff-е:

0970_hacker_comment_ru/image2.png

Минус 12 строк. Это, кстати, пересекается со статьёй "Предупреждения помогают писать лаконичный код". Неплохое сокращение и упрощение функции, если, конечно, это не ошибка, и строк наоборот должно быть больше :).

Спасибо за внимание и приглашаю познакомиться с аналогичными заметками:

Популярные статьи по теме


Комментарии (0)

Следующие комментарии next comments
close comment form