False positive for "Out of bound memory access (accessed memory precedes memory block)" in C

Rules and Languages Report False-positive / False-negative...
1

versions used: SonarQube Scanner 4.2.0.1873

#include <stdlib.h>

void bogus(void) {
    char encoded[8];

    size_t encoded_size = sizeof(encoded) - 2;
    encode("foobar", encoded, &encoded_size);
    if (encoded_size > sizeof(encoded) - 2) {
        return;
    }
    encoded[encoded_size] = '\n';
    encoded[encoded_size + 1] = '\0';
}

The scanner complains that “Out of bound memory access (accessed memory precedes memory block)” online 12 (encoded[encoded_size + 1] = '\0';), even though the array index is explicitly checked on line 8. Clearly encoded_size` is between 0 and 6 (inclusive), so there cannot be an out of bound condition.

3

Hello @barbibulle,

The error message is not related to the if condition. It indicates that the memory you are accessing precedes the array. It is usually triggered when something like this is done:

void bogus(void) {
  char encoded[8];
  char* p = encoded;
  --p;
  p[0] = '\0';
}

If the index is higher than 7 you would get something like: access exceeds upper limit of memory block

I couldn’t reproduce the false-positive since I don’t have the implementation of encode. If you don’t call encode the message should disappear. My only guess is that encode is changing the value of encoded/encoded_size to cause is out of bound access. Can you share a full test case that reproduces the issue?

4

Hi @Abbas, the actually implementation of encode here doesn’t matter (that’s why I didn’t include it, and in my small test here, I run the scanner without any implementation for that function): the test if (encoded_size > sizeof(encoded) - 2) ensures that encoded_size isn’t pointing past the bytes in the encoded array, and since encoded_size is unsigned, the memory access couldn’t precede the array either.

5

Hello @barbibulle, the implementation of encode here will help detect the reason for the possible false-positive. As mentioned, I tried to reproduce the issue without encode and failed(no issue is raised). Can you share a full example that compiles that triggers this false-positive? Let me know if it is complicated to isolate a reproducer, we can look at other options to generate an automatic reproducer.

It doesn’t necessarily have to do with encoded_size. The access can precede the array if encoded is changed.

6

Actually in this minimal example I left encode unimplemented (just declared as an external function), and got the error.
I tried with an actual dummy implementation for encode (a function that does nothing: void encode(const char* msg, char* buffer, size_t* buffer_size) {}) and got this: if I put encode in a separate C file, I still get the error. But if I put encode in the same C file, I don’t get the error.
Also, about your comment that “It doesn’t necessarily have to do with encoded_size. The access can precede the array if encoded is changed”, in this small function, encoded can’t change, since it is a local variable and no reference to it is passed at anytime. So my comment about a preceding access not being possible still stands.

It is also weird that it complains about a preceding access only for the second statement (encoded[encoded_size + 1] = '\0';) but not for the first one (encoded[encoded_size] = '\n';), even though clearly the first index precedes the second one…

7

@barbibulle, my bad I was able to reproduce it by declaring without implementing encode. I created this ticket in order to fix it. This is a pretty serious bug, so hopefully, we are going to fix it by next release

Thanks a lot for reporting this and for the detailed explanation!

1 Like
9

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.