Does this usage of strncmp contain an out of bounds read?
Asked Answered
V

3

13

Fortify indicates that this is an out of bounds read:

if (strncmp("test string", "less than 32 char", 32) == 0)
{
...
}

It says that the function reads data from outside the bounds of less than 32 char.

Is there really a finding if strncmp goes beyond 32 chars and the second string is less than 32 chars?

Vocalic answered 10/8, 2016 at 15:51 Comment(10)
Fortify is wrong. At least for a proper, that is Standard conforming implementation of strncmp().Novella
Unless I am very much mistaken, strncmp() will only need to compare a single char from each of the strings in your example. I second the false positive.Eadith
Please provide a minimal reproducible example and see How to Ask. That would include the parametrisation of Fortify. Or ask HP why it generates a false positive with the required information.Ecclesiastic
@Olaf: I met those requirements. ThanksVocalic
Note that "test string" is not a string but a string literal. A string literal may have multiple null characters. A string has only one. IAC, this string literal certainly has at least one null character and does not pose any problem here.Nigger
Are you sure Fortify is not complaining about the code you did not post?Biz
Valgrind also often complains about out-of-bounds reads in strncmp, strlen, and other strxxx methods.Sphingosine
String function implementations may be performance optimized to process data in naturally aligned chunks of 4 or 8 bytes. The alignment guarantees that no extraneous memory exceptions will be triggered (every access is completely within one page) and is thus "safe", but this implementation technique can draw complaints from tools that check for access out of bounds, i.e. beyond the limits of a particular data object. My take is that such optimizations are allowed under the as-if rule, and thus compliant with the C standard.Centrality
@Centrality You might want to turn your comment into an answer. I had similar problems with strcmp, which were caused exactly by the problem you mentioned.Paroicous
@Paroicous Thanks for the endorsement, I have provided a full answer as you suggestedCentrality
E
10

TL;DR - strncmp() will keep comparing the string elements, until either the end of either string or 32 elements (characters), whichever is fewer.

A(ny) string is always null-terminated and upon encountering null-terminator, no further comparison is performed. Your code is safe.

Quoting C11, chapter §7.24.4.4 (emphasis mine)

int strncmp(const char *s1, const char *s2, size_t n);

The strncmp function compares not more than n characters (characters that follow a null character are not compared) from the array pointed to by s1 to the array pointed to by s2.

Endoblast answered 10/8, 2016 at 15:54 Comment(0)
C
13

For performance reasons, implementations of the standard string functions will often process the data in naturally aligned register-width chunks. This can cause read access past the end of the source data objects, but the alignment guarantees that the code behaves exactly like a naive implementation with respect to memory exceptions. Each wide access is contained within a single page, and no pages are touched that would not also be touched by a byte-wise implementation.

I would claim that such implementations are covered by C's as-if rule, that is, they behave the same "as if" they were following the abstract functional specifications.

An example of such an optimized implementation would be OpenSolaris's strcmp() for SPARC v8. This is code I wrote some fifteen years ago, along with other performance-optimized string functions.

Various memory checker tools will complain about such code, however, because its use can lead to access beyond the limits of the allocated data object, even though the out-of-bounds read access is harmless by design.

Centrality answered 10/8, 2016 at 19:56 Comment(1)
It's too bad that "modern" compiler writers show little interest in allowing programmers to use such techniques to perform tasks not built into the Standard Library. Sometimes a compiler may be able to find equivalent or better optimizations when given code that simply processes items sequentially, but in many cases compilers don't find such optimizations. Unfortunately, they may find UB-based "optimizations" that may randomly break user code that uses such techniques.Comose
E
10

TL;DR - strncmp() will keep comparing the string elements, until either the end of either string or 32 elements (characters), whichever is fewer.

A(ny) string is always null-terminated and upon encountering null-terminator, no further comparison is performed. Your code is safe.

Quoting C11, chapter §7.24.4.4 (emphasis mine)

int strncmp(const char *s1, const char *s2, size_t n);

The strncmp function compares not more than n characters (characters that follow a null character are not compared) from the array pointed to by s1 to the array pointed to by s2.

Endoblast answered 10/8, 2016 at 15:54 Comment(0)
L
6

You have perfectly valid code.

Either your compiler is generating bad object code or fortify is reporting a false positive.

I doubt it is the compiler generating bad code. That will create too many problems and will be detected and fixed in no time.

It is most likely that fortify is reporting a false positive.

Ligialignaloes answered 10/8, 2016 at 15:54 Comment(1)
Generating what bad object code? For what? A function call? to a standard library function? This is a source code analysis. Nothing to do with the compiler or object code whatsoever.Tendance

© 2022 - 2024 — McMap. All rights reserved.