SonarLint issues cpp:S5782 for correct code when calling wmemcmp

Clean Code Report False-positive / False-negative...
, ,
1
  • Operating system: Windows 10 64-bit
  • SonarLint plugin version: SonarLint for Visual Studio 2022 version 6.15.0.67199
  • Programming language you’re coding in: C++ (native)
  • Is connected mode used: No, not connected - even though we do license SonarQube

SonarLint incorrectly issues a cpp:S5782 (“POSIX functions should not be called with arguments that trigger buffer overflows”) when comparing buffers using wmemcmp. Specifically, when the second parameter to wmemcmp - the read buffer - is a literal wchar_t array, SonarLint calculates the length of that read buffer as one less than it should.

E.G. a buffer of L"Test" is calculated as 9 bytes long: 8 bytes for the four wchar_t characters in Test (correct), plus ONLY ONE byte for the null terminating character. However, SonarLint should be accounting for TWO bytes for the null terminating character because it is a WIDE (16-bit) null terminating character: L'\0', rather than the one-byte null terminating character for a narrow char array: '\0'.

Here is a simple 15-line C++ function to replicate this issue easily:

#include <iostream>

int testBuffers()
{
	wchar_t buffer[5]{ L'T', L'e' , L's' , L't' , L'\0' };

	if (wmemcmp(buffer, L"Test", 5) != 0)  // SonarLint issues cpp:S5782 on this line, when it should not
	{
		std::wcout << L"ERROR - buffer does not match wide string 'Test'\n";
		return 1;
	}

	std::wcout << L"buffer matches wide string 'Test'\n";
	return 0;
}

In the above function, the wmemcmp call succeeds and compares the two buffers as equal - even though SonarLint mistakenly warns that the size parameter (which is expressed in number of wide characters rather than bytes) exceeds the bounds of the read buffer by one byte (it doesn’t).

In the SonarSource Jira project (which I can only view), there are two related issues:

S5782: FP on wcsncmp and other c-str functions for null-terminated strings

# S5782 assumes incorrect size of the source buffer passed to wcsnrtombs

I suspect that if SonarLint was corrected to account for TWO bytes for each WIDE string null terminating character, that my issue would be resolved, and that the above two Jira issues would be resolved or at least simplified.

1 Like
2

Hi,

Version 6.16.0.69538 was released last week. Would you mind upgrading and making sure this is still replicable, please?

 
Thx,
Ann

3

I upgraded SonarLint for Visual Studio 2022 to version 6.16.0.69538, and the issue can still be reproduced.

1 Like
7

Hi @MikeOnline and welcome to the community!

Thank you for the great description of the FP and your analysis.

You’ve correctly identified the root cause: we miscompute the length of a wide-character string literal. Indeed, it could be fixed by “S5782: FP on wcsncmp and other c-str functions for null-terminated strings” as well, even though the underlying issue here is different.

I have created a ticket CPP-4428 to track your reported FP.

1 Like
9

Arseniy,

Thanks for confirming my findings and opening the new ticket.

You’ve correctly identified the root cause: we miscompute the length of a wide-character string literal.

In the example C++ function I provided, buffer is initialized with 5 wchar_t literals. But buffer itself is not a wide-character string literal. buffer is a non-const variable that could be modified to hold different wide strings (though its maximum length is fixed). So SonarLint may also be miscomputing the length (in bytes) of any array of wide characters – not just wide character string literals such as L"Test". I have not tested SonarLint with buffers based on std::vector<wchar_t>, std::array<wchar_t, 5>, etc.

I assume this issue will be resolved once SonarLint expects any wide character literal or any array or buffer used in functions taking a null-terminated wide string to end with a wide null (2 bytes).

Thanks again.

1 Like