Новые диагностические правила в PVS-Studio 7.34

• С++
  • V1116
  • V1117 [Для языка C]
  • V2022 [Для языка C]
  • V5014 [Стандарт OWASP]
• C#
  • V3207
  • V3208 [Unity Engine]
  • V3209 [Unity Engine]
  • V3210 [Unity Engine]
  • V4007 [Unity Engine]
  • One more thing...
• Java
  • V6123
  • V6124
  • V6125
  • V5309 [Стандарт OWASP]

С релизом PVS-Studio 7.34 в анализаторе появились новые диагностические правила: taint для Java, множество Unity-диагностик для C#, углубление в OWASP и многое другое! Расскажем о них в этой статье.

С++

Команда C++ в новом релизе постаралась охватить диагностики общего назначения и поддержку различных стандартов разработки ПО.

Но это только начало! В планах команды охватить ещё больше диагностик по стандарту MISRA, ждите новостей :)

А сейчас пройдёмся по основным диагностикам релиза 7.34.

V1116

Creating an exception object without an explanatory message may result in insufficient logging.

Новое диагностическое правило предназначено для обнаружения ситуаций, когда исключения создаются без поясняющих сообщений.

Отсутствие сообщения может затруднить процесс выявления и исправления ошибок, а также общую читаемость кода.

Пример кода, на котором анализатор PVS-Studio сгенерирует предупреждение:

void SomeCheck(const char *val)
{
  if (!val) throw std::runtime_error { "" };
  ....
}

void Foo()
{
  const char *val = ....;
  try
  {
    SomeCheck(val);              // <=
  }
  catch(std::runtime_error &err)
  {
    std::cerr << err.what() << std::endl;
  }
}

В функции SomeCheck в случае возникновения ошибки выбрасывается исключение с пустым сообщением, которое будет обработано в функции Foo. В процессе обработки ожидается, что в std::cerr попадёт информация о причине создания исключения, но этого не происходит.

Создавая код таким образом, разработчик передаёт коллегам пожелания "счастливой отладки". Это затрудняет понимание того, что именно вызвало сбой.

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

V1117 [Для языка C]

The declared function type is cv-qualified. The behavior when using this type is undefined.

А это диагностическое правило только для языка C.

Оно направлено на выявление случаев определения типа функции с использованием квалификаторов const или volatile.

Согласно стандарту С23 (пункту 10 параграфа 6.7.4.1), при использовании таких типов поведение будет не определено.

Пример кода, на котором анализатор PVS-Studio сгенерирует предупреждение:

typedef int fun_t(void);

typedef const fun_t const_qual_fun_t;          // <=

typedef const fun_t * ptr_to_const_qual_fun_t; // <=

void foo()
{
  const fun_t c_fun_t;       // <=
  const fun_t * ptr_c_fun_t; // <=
}

V2022 [Для языка C]

Implicit type conversion from integer type to enum type.

И ещё одна диагностика для языка C, которая может помочь во время рефакторинга и отладки.

Это правило позволяет анализатору выявлять неявные приведения целочисленных типов к типам перечислений.

Пример кода, на котором анализатор PVS-Studio сгенерирует предупреждение:

Orientation GetOrientation (bool b)
{
  int posOne = 1;
  int posTwo = 2;
  return b ? posOne : posTwo;    // V2022
}

В этом коде используется условный оператор (?:) для выбора между двумя целочисленными переменными posOne и posTwo, что приводит к неявному приведению.

V5014 [Стандарт OWASP]

OWASP. Cryptographic function is deprecated. Its use can lead to security issues. Consider switching to an equivalent newer function.

Новая диагностика SAST направления в части защищённости.

Это диагностическое правило разработано согласно стандарту безопасной разработки OWASP.

Оно направлено на выявление вызовов устаревшей криптографической функции. Их использование может повлечь за собой серьёзные проблемы с безопасностью ПО.

Пример кода, на котором анализатор PVS-Studio сгенерирует предупреждение:

BOOL ImportKey(HCRYPTPROV hProv, LPBYTE pbKeyBlob, DWORD dwBlobLen)
{
  HCRYPTKEY hPubKey;
  if (!CryptImportKey(hProv, pbKeyBlob, dwBlobLen, 0, 0, &hPubKey))
  {
    return FALSE;
  }
  if (!CryptDestroyKey(hPubKey))
  {
    return FALSE;
  }
  return TRUE;
}

Согласно документации Microsoft, функции CryptoImportKey и CryptoDestroyKey устарели. Их следует заменить на безопасные аналоги из Cryptography Next Generation (BCryptoImportKey и BCryptoDestroyKey).

А ещё ошибки, выявленные этой диагностикой, классифицируются согласно ГОСТ Р 71207–2024 как критические и относятся к типу "Ошибки некорректного использования системных процедур и интерфейсов, связанных с обеспечением информационной безопасности (шифрования, разграничения доступа и пр.)".

Но это только начало! В планах C и C++ команды охватить ещё больше диагностик по различным стандартам разработки ПО. Особое внимание будет уделено стандарту MISRA. В общем, ждите новостей :)

C#

В новом релизе PVS-Studio 7.34 команда C# анализатора сфокусировалась на создании Unity-специфичных диагностических правил, но и про диагностики общего назначения не забыла.

С такой и начнём.

V3207

The 'not A or B' logical pattern may not work as expected. The 'not' pattern is matched only to the first expression from the 'or' pattern.

Новое диагностическое правило направлено на выявление некорректного использования паттерна not A or B. Проблема связанна с путаницей разработчиков в порядке операций.

Пример кода, на котором анализатор PVS-Studio сгенерирует предупреждение:

private void ShowWordDetails(string key)
{
  if (key is not "" or null)
  {
    PanelReferenceBox.Controls.Clear();

    CurrentWord = Words.Find(x => x.Name == key);
    ....
  }
}

В начале метода присутствует проверка входного параметра key на пустую строку или null.

Но в логике условного выражения была допущена ошибка. Всё дело в том, что приоритет оператора not выше, чем у оператора or. Вследствие этого отрицание не применяется к правой части выражения, и, если key будет иметь значение null, то условие будет true.

V3208 [Unity Engine]

Unity Engine. Using 'WeakReference' with 'UnityEngine.Object' is not supported. GC will not reclaim the object's memory because it is linked to a native object.

А вот и первая диагностика из нового цикла Unity-специфичных правил.

Она направлена на поиск мест с использованием UnityEngine.Object (или других объектов, наследуемых от него) вместе с System.WeakReference.

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

Пример кода, на котором анализатор PVS-Studio сгенерирует предупреждение:

WeakReference<GameObject> _goWeakRef;
void UnityObjectWeakReference()
{
  var go = new GameObject();
  _goWeakRef = new WeakReference<GameObject>(go);
}

В примере создаётся слабая ссылка на объект класса GameObject. Даже если вы не создавали сильных ссылок на этот объект, сборщик мусора не сможет очистить его.

V3209 [Unity Engine]

Unity Engine. Using await on 'Awaitable' object more than once can lead to exception or deadlock, as such objects are returned to the pool after being awaited.

В ещё одной диагностике для Unity анализатор ищет места с несколькими использованиями одного и того же UnityEngine.Awaitable объекта с оператором await.

Для оптимизации Awaitable объекты хранятся в пуле объектов.

При await-вызове объект Awaitable возвращается в пул. После этого при повторном применении await к этому же объекту будет выброшено исключение. В некоторых случаях также возможна взаимная блокировка.

Пример кода, на котором анализатор PVS-Studio сгенерирует предупреждение:

async Awaitable<bool> AwaitableFoo() { .... }

async Awaitable ExampleFoo()
{
  Awaitable<bool> awaitable = AwaitableFoo();
  if (await awaitable)
  {
    var result = await awaitable;
    ....
  }
}

В этом коде будет выброшено исключение (или возникнет взаимная блокировка) при инициализации переменной result значением, полученным с помощью await-вызова awaitable. Это произойдёт из-за того, что ранее await уже применялся к awaitable в условной конструкции.

V3210 [Unity Engine]

Unity Engine. Unity does not allow removing the 'Transform' component using 'Destroy' or 'DestroyImmediate' methods. The method call will be ignored.

Это диагностическое правило направлено на выявление аномалий, связанных с вызовом методов Destroy или DestroyImmediate класса UnityEngine.Object.

Проблема возникает в ситуации, если используется аргумент типа UnityEngine.Transform, это приводит к ошибке во время вызова метода. Удаление компонента Transform из игрового объекта в Unity не допускается.

Пример кода, на котором анализатор PVS-Studio сгенерирует предупреждение:

using UnityEngine;

class Projectile : MonoBehaviour
{
  public void Update() 
  {
    if (....)
    {
      Destroy(transform);
    }
    ....
  }
}

Свойство transform из базового класса MonoBehaviour возвращает экземпляр класса Transform, который передаётся в качестве аргумента методу Destroy.

При таком вызове метода Unity выдаст сообщение об ошибке, а сам компонент не будет уничтожен.

V4007 [Unity Engine]

Unity Engine. Avoid creating and destroying UnityEngine objects in performance-sensitive context. Consider activating and deactivating them instead.

Эта диагностика уже направлена на другой спектр ошибок — проблемы с производительностью.

Задача этого правила — помочь анализатору обнаружить создание Unity-объектов в часто выполняемом методе.

Регулярное создание/уничтожение игровых объектов не только нагружает центральный процессор, но и приводит к увеличению частоты вызовов сборщика мусора. Это негативно отражается на производительности.

Пример кода, на котором анализатор PVS-Studio сгенерирует предупреждение:

CustomObject _instance;

void Update()
{
  if (....)
  {
     CreateCustomObject();
     ....
  }
  else if (....)
  {
    ....
    Destroy(_instance.gameObject);       // <=
  }
}

void CreateCustomObject()
{
  var go = new GameObject();             // <=
  _instance = go.AddComponent<CustomObject>();
  ....
}

Здесь в методе Update создаётся и уничтожается некоторый игровой объект _instance. Так как Update выполняется при каждом обновлении кадров, рекомендуется по возможности избегать в нём этих операций.

Но это ещё не все Unity-диагностики! Скоро их станет ещё больше, так что ждите новостей от нашей команды :)

One more thing...

Мы не можем не рассказать про одну важную доработку в C# анализаторе — отслеживание изменения возвращаемого значения метода между вызовами. Но что это меняет? Давайте разберёмся.

Взглянем на такой пример:

void Example()
{
  if (Foo() != null)
  {
    var value = Foo().Value;
  }
}

Value Foo()
{
  if (_condition)
  {
    ....
    return value;
  }
  return null;
}

В методе Example() выполняется проверка возвращаемого значения Foo() на неравенство null. После чего в теле условия метод Foo() вызывается повторно, и его возвращаемое значение разыменовывается.

Раньше анализатор выдал бы тут предупреждение, потому что не учитывал контекст инвокации и смотрел только на код её декларации, а там есть возможность возврата null.

Теперь анализатор понимает, что в обоих случаях Foo() возвращает одно и то же значение, и предупреждения не будет.

Но давайте взглянем на пример с немного изменённым кодом...

void Example()
{
  if (Foo() != null)
  {
    _condition = false;      // <=
    var value = Foo().Value;
  }
}

Value Foo()
{
  if (_condition)
  {
    ....
    return value;
  }
  return null;
}

Из декларации метода Foo() можно понять, что когда _condition == true — метод возвращает не null.

Анализатор увидит изменение поля _condition перед второй инвокацией и сделает предположение: если используемое внутри Foo() поле изменилось, то и возвращаемое значение Foo() могло измениться.

В результате чего срабатывания на потенциальное разыменование останутся.

Java

С релизом PVS-Studio 7.34 в Java анализаторе появился механизм для проведения taint-анализа!

На его основе была создана первая диагностика — поиск SQL-инъекций. В будущих обновлениях в Java анализаторе будет сделан упор на SAST, покрытие списка наиболее распространённых потенциальных уязвимостей OWASP Top-10, а также будет добавлено больше taint-диагностик.

Но начнём с новых диагностик общего назначения, они тоже достаточно интересные.

V6123

Modified value of the operand is not used after the increment/decrement operation.

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

Проблема заключается в том, что либо операция избыточна, либо, что более серьёзно, операции перепутали и хотели использовать префиксную.

Пример кода, на котором анализатор PVS-Studio сгенерирует предупреждение:

int calculateSomething() {
  int value = getSomething();
  ....
  return value++;
}

Оператор ++ никак не повлияет на значение, которое вернёт метод calculateSomething.

V6124

Converting an integer literal to the type with a smaller value range will result in overflow.

Как можно понять из названия этой диагностики, её цель — выявить возможное переполнение.

Пример кода, на котором анализатор PVS-Studio сгенерирует предупреждение:

public static void test() {
  byte a = (byte) 256;       // a = 0
  short b = (short) 32768;   // b = -32768
  int c = (int) 2147483648L; // c = -2147483648
}

Переменной целочисленного типа присвоили значение, выходящее за диапазон допустимых значений, из-за чего произойдёт переполнение.

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

А ещё ошибки, выявленные этой диагностикой, классифицируются согласно ГОСТ Р 71207–2024 как критические и относятся к типу "Ошибки целочисленного переполнения и некорректного совместного использования знаковых и беззнаковых чисел".

V6125

Calling the 'wait', 'notify', and 'notifyAll' methods outside of synchronized context will lead to 'IllegalMonitorStateException'.

А эта диагностика помогает выявить проблемы с синхронизацией.

Пример кода, на котором анализатор PVS-Studio сгенерирует предупреждение:

public void someMethod() {
    notify();
}

public void anotherMethod() throws InterruptedException {
    wait();
}

Анализатор обращает внимание на методы wait, notify, notifyAll, так как они могут быть вызваны в несинхронизированном контексте. Они работают с монитором объекта, по которому происходит синхронизация. То есть их вызов корректен только в синхронизированном контексте и только на объекте, по которому происходит синхронизация.

В случае, если методы wait, notify или notifyAll вызвать в несинхронизированном контексте или не на том объекте, по которому синхронизация происходит, произойдёт выброс исключения IllegalMonitorStateException.

V5309 [Стандарт OWASP]

OWASP. Possible SQL injection. Potentially tainted data is used to create SQL command.

Первая taint-диагностика Java анализатора! Если конкретнее, то теперь анализатор может выявлять потенциальные SQL-инъекции.

SQL-инъекция — уязвимость, позволяющая злоумышленнику внедрить свой код в SQL-запрос. Если в запросе используются данные извне, то без их корректной проверки вы рискуете целостностью и конфиденциальностью информации, хранящейся в вашей базе данных.

@Autowired
private JdbcTemplate template;

@GetMapping("/get_all_secret_data")
public void getEndpoint(@RequestParam String param) {
    var sql = "SELECT * FROM Users WHERE id = '" + param + "'";
    template.execute(sql);
    ....
}

В случае, если пользователь окажется злоумышленником и значение param будет примерно следующим: - "111' or 1=1; drop table users; select ' " — с таблицей users можно будет попрощаться. Поэтому важно проверять данные, приходящие извне.

Спасибо за прочтение!

Если у вас есть пожелания по статьям или вопросы, то можете отправить их в форме обратной связи. И, конечно, ждём вас в комментариях :)