False Positive on sonar qube with C language

SonarQube Server / Community Build
, , ,
1

We are using sonarQube 10.4.
we are getting a finding from sonarqube that seem to me a false positive:
sonar give a finding in the line password[strcspn(password, “\n”)] = ‘\0’

out of bound memory access (indez is tainted)

I can’t understand why:for me this code can’t cause the buffer overflow

here the full code:

void myTestFunction()
{
    char password[1024] = {};

    FILE *file = fopen("myFile", "r");
    if(file == NULL)
    {
        return;
    }
    if(fgets(password, sizeof(password), file) == NULL)
    {
        CGC_LOGERROR("CMT", "Failed to get password file content");
    }
    else
    {
        password[strcspn(password, "\n")] = '\0';
    }

    fclose(file);
}

**

sonar_issue
sonar_issue962×811 82.7 KB

**

2

Hi,

Which rule is raising this issue? The rule ID will look something like S123.

Also, your screenshot seems to be from SonarLint. Just to verify, you’re in connected mode to a SonarQube 10.4 server?

 
Thx,
Ann

3

Thank you Ann for replaying.
The rose rule is c:S3519.

I’m in connected mode, I’m using sonarlint in eclipse (connect mode is the only way to have C language support on eclipse sonarlint ).

I’ve got the same finding in the sonarQube WEB interface.

5

Hi,

Thanks for the rule ID. I’ve flagged this for the language experts.

 
Ann

9

Hi, Alessandro

Thank you for this report. I confirm that it is indeed a false positive, and I created a ticket to work on it: CPP-5829, which you can refer to to track our progress.

Here is what is going wrong with our analyzer in this case:

  • As you can see in the bug report, analyzer assumes that a malicious user can control “myFile” (taint origin) and through that the value stored in password and the return value of strcspn (taint propagation). This attacker-controlled value is then used as an index to a memory buffer password, which theoretically could cause out-of-bound memory access.
  • However, in this particular case, fgets guarantees a null terminator somewhere within password, so strcspn(password, ...) return value is bound by sizeof(password) and cannot cause out-of-bound memory access.
  • Our analyzer does not model fgets and strcspn accurately enough to recover the condition above and loses the upper bound on the accessed index.
3 Likes
12

Thank you @necto

Looking forward for the fix

14

Hi All
any news about it :slight_smile:

15

Hi Alessandro

At the moment, we have no specific plans regarding this false positive.

16

Hi Arseniy,

Thank you for the feedback

regards
Alessandro