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?

atk
  • 9,244
  • 3
  • 32
  • 32
Engineer2021
  • 3,288
  • 6
  • 29
  • 51
  • 2
    Fortify is wrong. At least for a proper, that is Standard conforming implementation of `strncmp()`. – alk Aug 10 '16 at 15:54
  • 1
    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. – EOF Aug 10 '16 at 15:56
  • Please provide a [mcve] and see [ask]. That would include the parametrisation of Fortify. Or ask HP why it generates a false positive with the required information. – too honest for this site Aug 10 '16 at 16:16
  • 5
    @Olaf: I met those requirements. Thanks – Engineer2021 Aug 10 '16 at 16:16
  • 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. – chux - Reinstate Monica Aug 10 '16 at 16:22
  • 1
    Are you sure Fortify is not complaining about the code you did not post? – chqrlie Aug 10 '16 at 16:42
  • Valgrind also often complains about out-of-bounds reads in strncmp, strlen, and other strxxx methods. – FredK Aug 10 '16 at 16:44
  • 3
    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. – njuffa Aug 10 '16 at 18:35
  • @njuffa 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. – anatolyg Aug 10 '16 at 18:59
  • @anatolyg Thanks for the endorsement, I have provided a full answer as you suggested – njuffa Aug 10 '16 at 19:57

3 Answers3

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.

njuffa
  • 23,970
  • 4
  • 78
  • 130
  • 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. – supercat Aug 10 '16 at 21:34
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.

Constantino Tsarouhas
  • 6,846
  • 6
  • 43
  • 54
Sourav Ghosh
  • 133,132
  • 16
  • 183
  • 261
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.

R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • 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. – user207421 Aug 11 '16 at 00:20