3

I have the code below. After running the cppcheck tool, it reports an error as Buffer is accessed out of bounds? An error is reported on line with the snprintf.

#include <stdio.h>

int main(int argc, char * argv[])
{
    if (argc > 1) {
        char testref[8] = "";
        snprintf(testref, sizeof(testref), "Ref:%s", argv[1]);
        printf("===>testref=%s\n", testref);
    }
}

below the command line interaction :

amin@ubuntu:$ gcc test.c -o test
amin@ubuntu:$ 
amin@ubuntu:$ ./test hello_world
===>testref=Ref:hel
amin@ubuntu:$ cppcheck test.c 
Checking test.c...
[test.c:7]: (error) Buffer is accessed out of bounds.
amin@ubuntu:$

Is cppcheck correct to report this error?

ABR
  • 125
  • 8
  • 1
    The value in `argv[1]` will be truncated if it is more than 3 bytes long, but the code has no other issues that I can see. A bug report is probably appropriate — or, at least, a request for why it isn’t a bug in `cppcheck`. – Jonathan Leffler Jan 27 '20 at 22:55
  • 1
    Which version of cppcheck are you using? With Cppcheck 1.82 I don't get any error for this code. – wovano Jan 27 '20 at 23:21
  • 1
    @wovano I'm using the version : Cppcheck 1.72 – ABR Jan 27 '20 at 23:27
  • 1
    `snprintf()` returns a value. Use it! – wildplasser Jan 27 '20 at 23:30
  • 1
    Could you update Cppcheck and see if the error remains? – wovano Jan 27 '20 at 23:41
  • IMHO your time is better spent reading man pages than using code checkers or debuggers. (or consulting SO). – wildplasser Jan 28 '20 at 00:01
  • 2
    Had `snprintf()` returned -1, then `printf("===>testref=%s\n", testref);` is _not_ a good thing to do. For completeness. report the return value of `snprintf()` as suggested well by @wildplasser – chux - Reinstate Monica Jan 28 '20 at 00:19
  • 3
    @wildplasser, man pages are very useful indeed, but how do you think this will help with this specific question? (If you think you have the solution, please consider posting an answer.) – wovano Jan 28 '20 at 00:27
  • Before c99, implementations of `snprintf()` existed that always returned -1 on potential overrun. After c99, you "only" have to check for `retval >= sizeof buff`. As I said: dont waste your time debugging, read the fine manual instead. – wildplasser Jan 28 '20 at 00:27
  • 2
    @wildplasser, I have searched for documentation about this, but couldn't find it. Current man pages refer to the "correct" behavior where AFAIK no buffer overruns can occur. In the original ANSI standard the snprintf() function didn't exist. I would appreciate a reference to the function you mention. That would indeed explain the observed issue. – wovano Jan 28 '20 at 00:29
  • 1
    True64/DigitalUnix had `snprintf()` before c99. With different semantics. You still had to check the return value. – wildplasser Jan 28 '20 at 00:34
  • 1
    @wildplasser I read the manpage for `snprintf()` and `cppcheck` carefully and can't find an explanation for the behaviour that the OP observed, either. Hence, I think your CV is inappropriate. – Ctx Jan 28 '20 at 01:02
  • 1
    The ticket [FP : buffer access out of bounds of array initialized by C string literal](https://trac.cppcheck.net/ticket/7283) in the Cppcheck bug tracker looks related. I suggest updating Cppcheck to a more recent version. The latest release is version 1.90. There are so many things that were improved, fixed or added (including new bugs ;) ). – versat Jan 28 '20 at 07:19

1 Answers1

2

I think, generally speaking, cppcheck is correct to report this error. The behavior of the snprintf function is implementation-dependent, and in some implementations it is not guaranteed that a null-character is written if the string is too large for the buffer. In such case, the consecutive call to printf() would read outside the boundaries of the buffer.

I could find at least one example of a snprintf implementation that would result in out-of-bound errors for your code. And according to this comment it was also the case for True64/DigitalUnix before c99.

It would be interesting to see if cppcheck also reports an error for the following code (it should not report an error):

#include <stdio.h>

int main(int argc, char * argv[])
{
    if (argc > 1) {
        char testref[8] = "";
        int ret = snprintf(testref, sizeof(testref), "Ref:%s", argv[1]);
        if (ret >= 0) {
            printf("===>testref=%s\n", testref);
        }
    }
}

Also note that Cppcheck version 1.82 does not report the error for your code. I'm not sure why version 1.72 does report the error and version 1.82 doesn't.

wovano
  • 4,543
  • 5
  • 22
  • 49
  • `snprintf()` is defined by the c standard, and these semantics should be considered for cppcheck. Thus, I would say that the new version 1.82 behaves correctly. – Ctx Jan 28 '20 at 00:58
  • @Ctx, there is no such thing as "the C standard" ;-) In the original ANSI standard this function was not present. In C99 it seems clearly defined, but there are many other standards and implementations. I'm not sure which standard cppcheck should consider.though. – wovano Jan 28 '20 at 01:30
  • 1
    cppcheck does have the option to pass in which standard it should be checking against. I'd hope that it would use the c99 version of `snprintf` if you pass it the `-std=c99` flag. – Christian Gibbons Jan 28 '20 at 16:09