New exciting diagnostic rules in PVS-Studio 7.35

• New C and C++ diagnostic rules
• New C# diagnostic rules
• New Java diagnostic rules

With the release of PVS-Studio 7.35, the analyzer got a ton of new diagnostic rules. You'll find lots of MISRA for C, new Unity diagnostic rules for C#, OWASP Top 10 coverage for Java, and so on. You can learn more in this note.

New C and C++ diagnostic rules

As we wrote in the press release, the C and C++ analyzer team has focused on the MISRA standard (you may notice this by the number of diagnostic rules).

Instead of dissecting each rule, though, let's go through the most interesting ones. To make the "selection" easier, I'd like to remind you that our new book is out! This is "C++ programmer's guide to undefined behavior", a kind of guide to undefined behavior and to its most secret and exotic corners. The book was written by Dmitry Sviridkin and edited by Andrey Karpov.

As you might guess from this foreword, we're offering you a tour of some diagnostic rules related to undefined behavior.

V2627. MISRA. Function type should not be type qualified. [For the C language]

Let's start with the first diagnostic rule that detects the possibility of undefined behavior.

The analyzer has detected that a function type is declared in the code using const, volatile, restrict, or _Atomic qualifiers. When such a type is used, the program behavior is undefined.

The code example where the analyzer issues a warning:

typedef int fun_t(void);

typedef const fun_t qual_fun_t;          // <=

typedef const fun_t * ptr_to_qual_fun_t; // <=

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

To ensure proper functionality, the const qualifier should be removed when declaring the function type.

V2631. MISRA. Pointers to variably-modified array types should not be used. [For the C language]

We continue our undefined journey...

In this diagnostic rule, the issue is related to the use of a variable-length array (VLA) pointer in object declarations or function parameter declarations. This can result in potential errors, so using the pointer is not recommended.

Undefined behavior occurs when two pointers to arrays of specified sizes are used in any context that requires them to be compatible, but their sizes differ.

The code example where the analyzer issues a warning:

void foo(int n, int (*a)[n])
{
  int (*b)[20] = a; // undefined behavior if n != 20
                    // types are assumed to be compatible
}

void bar(int n)
{
  int a[n];
  foo(n, &a);
}

void foobar()
{
  bar(10);
}

In this code, the foo function accepts the apointer to a VLA whose size is determined by the n parameter. Within the function, the pointer is assigned to another b pointer, which expects a pointer to an array of exactly 20 elements.

Such an operation leads to undefined behavior as long as n != 20.

Other diagnostic rules

As we said, the number of new MISRA diagnostic rules continues to grow, here are the rest:

V2626. MISRA C 2023 12.5. The sizeof operator should not have an operand which is a function parameter declared as 'array of type'.

V2628. MISRA C 2023 21.15. Pointer arguments to the Standard Library function 'Foo' should be pointers to qualified or unqualified versions of compatible types.

V2629. MISRA C 2023 21.16. Pointer arguments to the Standard Library function memcmp should point to either a pointer type, an essentially signed type, an essentially unsigned type, an essentially Boolean type or an essentially enum type.

V2630. MISRA C 2023 6.3. Bit field should not be declared as a member of a union.

V2632. MISRA C 2023 18.9. Object with temporary lifetime should not undergo array-to-pointer conversion.

V2633. MISRA C 2023 5.5. Identifiers should be distinct from macro names.

New C# diagnostic rules

In the previous PVS-Studio 7.34 release, the C# analyzer team promised to focus on creating Unity-specific diagnostic rules in the upcoming version of the analyzer.

Well, the guys weren't lying! Let's go through some of those rules.

V3214. Unity Engine. Using Unity API in the background thread may result in an error.

Let's start with a diagnostic rule that is new not only for our tool, but also for Unity, because it's related to the new Awaitable class.

The analyzer has detected that a property, method, or constructor is used after calling Awaitable.BackgroundThreadAsync. It may cause issues such as application hangs or throwing an exception when executed on a background thread.

The Unity documentation states that all APIs that interact with the engine must be used exclusively on the main thread.

The code example where the analyzer issues a warning:

private async Awaitable LoadSceneAndDoHeavyComputation()
{
  await Awaitable.BackgroundThreadAsync();
  await SceneManager.LoadSceneAsync("MainScene");
  ....
}

public async Awaitable Update()
{
  if (....)
    await LoadSceneAndDoHeavyComputation(); 

  ....
}

When the LoadSceneAndDoHeavyComputation method is executed, the call to the Awaitable.BackgroundThreadAsync method switches the subsequent code execution within the same method to the background thread.

This may cause issues when the SceneManager.LoadSceneAsync method is called later.

V3216. Unity Engine. Checking a field with a specific Unity Engine type for null may not work correctly due to implicit field initialization by the engine.

Here's another diagnostic rule but this time focusing on the non-obvious features of the Unity engine.

The analyzer has detected an unreliable null check for a field that can be initialized in the Unity inspector.

The code example where the analyzer issues a warning:

public class ActivateTrigger: MonoBehaviour
{
  [SerializeField]
  GameObject _target;

  private void DoActivateTrigger()
  {
     var target = _target ?? gameObject;
    ....
  }
}

In this case, if the _target value has not yet changed at runtime, the ?? check assumes _target is unequal to null, regardless of whether the field value has been assigned in the Unity inspector.

See the documentation for more details on why this is the case.

V3217. Possible overflow as a result of an arithmetic operation.

Specific diagnostic rules are a good thing, of course, but we can't forget about the general ones either!

This time the analyzer has detected an arithmetic operation that may result in an overflow.

The code example where the analyzer issues a warning:

private const int _halfMaximumValue = int.MaxValue / 2;

public void Calculate(int summand)
{
    int sum;

    if (summand > _halfMaximumValue + 1)
    {
        sum = _halfMaximumValue + summand;
    }

    ....
}

In the Calculate method, the sum of the passed parameter and the constant is calculated. The constant is half the maximum value of System.Int32. The parameter value is checked before the addition operation to avoid arithmetic overflow.

However, the condition contains an error: in our case, it checks whether summand is greater than _halfMaximumValue + 1. If the condition is true, the arithmetic overflow is guaranteed during the addition operation.

Other diagnostic rules

Do you think we're done talking about Unity? Not so fast!

Here are a few more Unity-specific diagnostic rules introduced in this release:

V3211. Unity Engine. The operators '?.', '??' and '??=' do not correctly handle destroyed objects derived from 'UnityEngine.Object'.

V3212. Unity Engine. Pattern matching does not correctly handle destroyed objects derived from 'UnityEngine.Object'.

V3213. Unity Engine. The 'GetComponent' method must be instantiated with a type that inherits from 'UnityEngine.Component'.

V3214. Unity Engine. Using Unity API in the background thread may result in an error.

V3215. Unity Engine. Passing a method name as a string literal into the 'StartCoroutine' is unreliable.

V3216. Unity Engine. Checking a field with a specific Unity Engine type for null may not work correctly due to implicit field initialization by the engine.

V3217. Possible overflow as a result of an arithmetic operation.

V4008. Unity Engine. Avoid using memory allocation Physics APIs in performance-sensitive context.

New Java diagnostic rules

On the version history page for the PVS-Studio 7.35 release, you may notice a paragraph that talks about the Java analyzer, "We are working on covering the OWASP Top 10 2021 standard for the Java analyzer. By this release, PVS-Studio for Java detects errors for 7 out of 10 categories".

Currently, PVS-Studio for Java detects vulnerabilities in the following categories:

• A01:2021-Broken Access Control;
• A02:2021-Cryptographic Failures;
• A03:2021-Injection;
• A04:2021-Insecure Design;
• A05:2021-Security Misconfiguration;
• A07:2021-Identification and Authentication Failures;
• A09:2021-Security Logging and Monitoring Failures.

I suggest you test this statement! Let's look at the diagnostic rules related to this:

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

The analyzer has detected that an operating system-level command is created from unverified external data. This can result in a command injection vulnerability.

The code example where the analyzer issues a warning:

public void doUsersCommand() throws IOException {
    Scanner sc = new Scanner(System.in);
    String command = sc.nextLine();
    Runtime.getRuntime().exec(command); 
}

The command string is external and is passed to the exec method as an OS-level command. Since the command is not validated before execution, it can contain any instructions, including malicious ones.

By the way, this diagnostic rule has a sibling, V5311. OWASP. Possible argument injection.

V5318. OWASP. Setting POSIX file permissions to 'all' or 'others' groups can lead to unintended access to files or directories.

The analyzer has detected files with unrestricted access permissions in the application. Specifically, the 'others' group gets the access. The 'others' group refers to all users and groups except the resource owner. Granting rights to this group may result in unauthorized access.

The code example where the analyzer issues a warning:

public void example() throws IOException {
    Path path = Path.of("/path/to/resource");
    Files.setPosixFilePermissions(
        path, 
        PosixFilePermissions.fromString("rwxrwxrwx")
    );
}

In this example, both the owner and all other users get access to the resource (reading, writing, and execution) by path. This violates the principle of least privilege.

It is essential to enforce maximum access restrictions on files and directories, especially if the resources contain confidential data.

Other diagnostic rules

OWASP diagnostics don't end there, here are some more:

V5312. OWASP. Possible XPath injection. Potentially tainted data is used to create XPath expression.

V5313. OWASP. Do not use the old versions of SSL/TLS protocols as it may cause security issues.

V5314. OWASP. Use of an outdated hash algorithm is not recommended.

V5315. OWASP. Use of an outdated cryptographic algorithm is not recommended.

V5316. OWASP. Do not use the 'HttpServletRequest.getRequestedSessionId' method because it uses a session ID provided by a client.

V5317. OWASP. Implementing a cryptographic algorithm is not advised because an attacker might break it.

V5319. OWASP. Possible log injection. Potentially tainted data is written into logs.

"Terrible tips for a C++ developer" in PDF

Now Andrey Karpov's book "Terrible tips for a C++ developer" is available in PDF. The book is written in a humorous format. It deals with real situations in C++ programming that devs should avoid.

The conditions for receiving the e-book are quite simple: subscribe to our article digest or Telegram bot. All details are at the link.

If you would like to get news on latest releases, subscribe to the PVS-Studio newsletter here.

Thank you for reading!

If you have questions or requests for articles, don't hesitate to send them via the feedback form. Last but not least, we'd love to hear your thoughts in the comments :)