Ignore possible memory leak on realloc-style C functions

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

Template for a good false-positive report, formatted with Markdown:

  • SonarQube Version: 8.3.1.34397
  • SonarScanner Version: 4.3.0.2102
... #standard includes
#include "mylib.h"

int main(...) {
   char *tmp = NULL;

   if ((tmp = strdup("This is a string"))) {
      tmp = my_strcat(tmp, " and this is some more");
      printf("Final String: %s\n", tmp);
      free(tmp);
   }
   return 1;
}

Sonar detects a possible memory leak with this code. While normally this would be correct, my library implements the function my_strcat by using realloc in the safe way (original pointer is stored, memory allocated, checks for failure, frees any needed pointers). Valgrind confirms that there are no memory leaks in this code at runtime.

Is there anyway to tell sonar to ignore this function for this specific rule? Obviously I could wrap the call in a regex ignore block, but I don’t want to have to update thousands of lines of code to ignore this very commonly used function.

2

Hello @KaibutsuX,

Are you using C or C++? Generally speaking giving the a function the same name as a standard function is a bad practice, specially if it is in the global namespace( if you are using C++).

Is it one place where you are getting the issue? you can always add // NOSONAR to make the analyzer ignore it.

3

@KaibutsuX, are you actually naming your function strcat or are you giving it another name? If you are giving it another name make sure you update to the latest CFamily 6.9.

4
  • C
  • 6.9.0 (build 17076) installed
  • no, see example “my_strcat”
  • Obviously I could wrap the call in a regex ignore block, but I don’t want to have to update thousands of lines of code to ignore this very commonly used function.
5

The reason why I asked this is that I don’t reproduce the issue on my side with “my_strcat”. I will need you to generate the reproducer file so I can reproduce what is happening on your side. To generate the reproducer file:

  • Add the reproducer option to the scanner configuration:
    sonar.cfamily.reproducer= "Full path to the .cpp file that has or include the file that has the false-positive"
  • Re-unning the scanner should generate a file named sonar-cfamily.reproducer in the project folder.
  • Please share this file. if you think this file contains private information we can send it privately.

Thanks,

6

Thanks I will get that generated.

When you’re reproducing are you including the source of ‘my_strcat’ in the project being analyzed? In my case my_strcat is a separate library to sonar has no access to see the function definition.

7

Attempted to run with reproducer:

$SONAR_SCANNER/bin/sonar-scanner -Dsonar.host.url=$SONAR_SERVER -Dsonar.projectKey=sonarTest -Dsonar.cfamily.threads=8 -Dsonar.login=$SONAR_TOKEN -Dsonar.cfamily.build-wrapper-output=sonar -Dsonar.branch.name=master -Dsonar.cfamily.reproducer=/home/quality/prj/sonarTest/main.c -X

Yields execution failure:

11:27:06.408 DEBUG: Using build-wrapper-dump.json probe
11:27:06.415 DEBUG: Using build-wrapper-dump.json probe
11:27:06.416 ERROR: 
The sonar.cfamily.reproducer property was set but no matching file was found. The property was set to:
  /home/quality/prj/sonarTest/main.c

11:27:06.450 INFO: ------------------------------------------------------------------------
11:27:06.450 INFO: EXECUTION FAILURE
11:27:06.450 INFO: ------------------------------------------------------------------------
11:27:06.451 INFO: Total time: 10.487s
11:27:06.509 INFO: Final Memory: 22M/84M
11:27:06.509 INFO: ------------------------------------------------------------------------
11:27:06.509 ERROR: Error during SonarScanner execution
java.lang.IllegalStateException: 
The sonar.cfamily.reproducer property was set but no matching file was found. The property was set to:
  /home/quality/prj/sonarTest/main.c

        at com.sonar.cpp.plugin.CFamilySensor.execute(CFamilySensor.java:327)
        at org.sonar.scanner.sensor.AbstractSensorWrapper.analyse(AbstractSensorWrapper.java:48)
        at org.sonar.scanner.sensor.ModuleSensorsExecutor.execute(ModuleSensorsExecutor.java:85)
        at org.sonar.scanner.sensor.ModuleSensorsExecutor.execute(ModuleSensorsExecutor.java:62)
        at org.sonar.scanner.scan.ModuleScanContainer.doAfterStart(ModuleScanContainer.java:82)
        at org.sonar.core.platform.ComponentContainer.startComponents(ComponentContainer.java:137)
        at org.sonar.core.platform.ComponentContainer.execute(ComponentContainer.java:123)
        at org.sonar.scanner.scan.ProjectScanContainer.scan(ProjectScanContainer.java:386)
        at org.sonar.scanner.scan.ProjectScanContainer.scanRecursively(ProjectScanContainer.java:382)
        at org.sonar.scanner.scan.ProjectScanContainer.doAfterStart(ProjectScanContainer.java:351)
        at org.sonar.core.platform.ComponentContainer.startComponents(ComponentContainer.java:137)
        at org.sonar.core.platform.ComponentContainer.execute(ComponentContainer.java:123)
        at org.sonar.scanner.bootstrap.GlobalContainer.doAfterStart(GlobalContainer.java:141)
        at org.sonar.core.platform.ComponentContainer.startComponents(ComponentContainer.java:137)
        at org.sonar.core.platform.ComponentContainer.execute(ComponentContainer.java:123)
        at org.sonar.batch.bootstrapper.Batch.doExecute(Batch.java:72)
        at org.sonar.batch.bootstrapper.Batch.execute(Batch.java:66)
        at org.sonarsource.scanner.api.internal.batch.BatchIsolatedLauncher.execute(BatchIsolatedLauncher.java:46)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
        at java.base/java.lang.reflect.Method.invoke(Unknown Source)
        at org.sonarsource.scanner.api.internal.IsolatedLauncherProxy.invoke(IsolatedLauncherProxy.java:60)
        at com.sun.proxy.$Proxy0.execute(Unknown Source)
        at org.sonarsource.scanner.api.EmbeddedScanner.doExecute(EmbeddedScanner.java:189)
        at org.sonarsource.scanner.api.EmbeddedScanner.execute(EmbeddedScanner.java:138)
        at org.sonarsource.scanner.cli.Main.execute(Main.java:112)
        at org.sonarsource.scanner.cli.Main.execute(Main.java:75)
        at org.sonarsource.scanner.cli.Main.main(Main.java:61)
make: *** [sonar] Error 1

And:

ls -lh /home/quality/prj/sonarTest/main.c
-rw-r--r-- 1 quality quality 1.3K May 27 14:18 /home/quality/prj/sonarTest/main.c
8

@KaibutsuX Is main.c built in the command you are passing to build-wrapper?

I would guess that the generated build-wrapper .json file doesn’t contain the file that you are trying to reproduce and that why it fails.

9

That is fine, the most important part is to set it the exact file that lead to the false-positive. This way we can make sure that we reproduce the exact same enviroment.

10

My build wrapper command is:
$SONAR_BUILD_WRAPPER/build-wrapper-linux-x86-64 --out-dir sonar make all

The make rule for all is:
gcc -m32 main.c -I…/mylib/include -L…/mylib/build/el6-32/lib -lmylib

  • Build wrapper version: build-wrapper, version 6.9 (linux-x86)

More debug from the scan:

11:56:23.992 INFO: Indexing files...
11:56:23.993 INFO: Project configuration:
11:56:24.012 DEBUG: 'main.c' indexed with language 'c'
11:56:24.014 DEBUG: 'scan.sh' indexed with no language
11:56:24.016 DEBUG: 'Makefile' indexed with no language
11:56:24.018 DEBUG: 'sonar/build-wrapper.log' indexed with no language
11:56:24.020 DEBUG: 'sonar/build-wrapper-dump.json' indexed with no language
11:56:24.022 DEBUG: 'a.out' indexed with no language
11:56:24.024 INFO: 6 files indexed

What I think is the relevant portion of the build-wrapper-dump.json:

{
"compiler":"clang",
"cwd":"/ssd/quality/prj/sonarTest",
"executable":"/usr/bin/gcc",
"cmd":[
"/usr/bin/gcc",
"-m32",
"main.c",
"-DSTRHASLEN_MACRO=1",
"-I../mylib/include",
"-L../mylib/build/el6-32/lib",
"-lmylib"],
"env":[
"SQ_WRAPPER_SOCKET=/tmp/build-wrapper-socket.Yrz3rv",
"SQ_WRAPPER_LIBRARY=/opt/build-wrapper-linux-x86/libinterceptor-${PLATFORM}.so",
"QTDIR=/usr/lib64/qt-3.3",
"SHELL=/bin/bash",
"_=/opt/build-wrapper-linux-x86/build-wrapper-linux-x86-64",
"SUDO_COMMAND=/bin/su quality",
"USERNAME=root",
"LESSOPEN=||/usr/bin/lesspipe.sh %s",
"PATH=/usr/local/bin:/ssd/quality/bin:/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/bin",
"LS_COLORS=rs=0:di=38;5;27:ln=38;5;51:mh=44;38;5;15:pi=40;38;5;11:so=38;5;13:do=38;5;5:bd=48;5;232;38;5;11:cd=48;5;232;38;5;3:or=48;5;232;38;5;9:mi=05;48;5;232;38;5;15:su=48;5;196;38;5;15:sg=48;5;11;38;5;16:ca=48;5;196;38;5;226:tw=48;5;10;38;5;16:ow=48;5;10;38;5;21:st=48;5;21;38;5;15:ex=38;5;34:*.tar=38;5;9:*.tgz=38;5;9:*.arj=38;5;9:*.taz=38;5;9:*.lzh=38;5;9:*.lzma=38;5;9:*.tlz=38;5;9:*.txz=38;5;9:*.zip=38;5;9:*.z=38;5;9:*.Z=38;5;9:*.dz=38;5;9:*.gz=38;5;9:*.lz=38;5;9:*.xz=38;5;9:*.bz2=38;5;9:*.tbz=38;5;9:*.tbz2=38;5;9:*.bz=38;5;9:*.tz=38;5;9:*.deb=38;5;9:*.rpm=38;5;9:*.jar=38;5;9:*.rar=38;5;9:*.ace=38;5;9:*.zoo=38;5;9:*.cpio=38;5;9:*.7z=38;5;9:*.rz=38;5;9:*.jpg=38;5;13:*.jpeg=38;5;13:*.gif=38;5;13:*.bmp=38;5;13:*.pbm=38;5;13:*.pgm=38;5;13:*.ppm=38;5;13:*.tga=38;5;13:*.xbm=38;5;13:*.xpm=38;5;13:*.tif=38;5;13:*.tiff=38;5;13:*.png=38;5;13:*.svg=38;5;13:*.svgz=38;5;13:*.mng=38;5;13:*.pcx=38;5;13:*.mov=38;5;13:*.mpg=38;5;13:*.mpeg=38;5;13:*.m2v=38;5;13:*.mkv=38;5;13:*.ogm=38;5;13:*.mp4=38;5;13:*.m4v=38;5;13:*.mp4v=38;5;13:*.vob=38;5;13:*.qt=38;5;13:*.nuv=38;5;13:*.wmv=38;5;13:*.asf=38;5;13:*.rm=38;5;13:*.rmvb=38;5;13:*.flc=38;5;13:*.avi=38;5;13:*.fli=38;5;13:*.flv=38;5;13:*.gl=38;5;13:*.dl=38;5;13:*.xcf=38;5;13:*.xwd=38;5;13:*.yuv=38;5;13:*.cgm=38;5;13:*.emf=38;5;13:*.axv=38;5;13:*.anx=38;5;13:*.ogv=38;5;13:*.ogx=38;5;13:*.aac=38;5;45:*.au=38;5;45:*.flac=38;5;45:*.mid=38;5;45:*.midi=38;5;45:*.mka=38;5;45:*.mp3=38;5;45:*.mpc=38;5;45:*.ogg=38;5;45:*.ra=38;5;45:*.wav=38;5;45:*.axa=38;5;45:*.oga=38;5;45:*.spx=38;5;45:*.xspf=38;5;45:",
"PWD=/ssd/quality/prj/sonarTest",
"G_BROKEN_FILENAMES=1",
"HOME=/ssd/quality",
"LOGNAME=quality",
"SUDO_GID=480",
"SHLVL=2",
"SUDO_UID=7202",
"USER=quality",
"CVS_RSH=ssh",
"MAKEFLAGS=w",
"MFLAGS=-w",
"HISTSIZE=1000",
"LANG=en_US.UTF-8",
"TERM=xterm-256color",
"MAKELEVEL=2",
"LD_PRELOAD="]}]}

I can see that sonar knows about main.c from the json and the analysis output, but full path or just main.c throws the same error.

11

@KaibutsuX looking at your build-wrapper file the directory of the analyzed main.c is /ssd/quality/prj/sonarTest. So the reproducer option should be sonar.cfamily.reproducer=/ssd/quality/prj/sonarTest/main.c

Thanks,

12

ahh, /home is symlinked to /ssd it must not be picking that up. I’ll try that, thanks.

13

Attached sonar reproducer file

Source code for mylib::my_strcat:

#include <stdio.h>                                                                                                                                                                                                                            
#include <stdlib.h>                                                                                                                                                                                                                           
#include <string.h>                                                                                                                                                                                                                           
                                                                                                                                                                                                                                              
char *my_strcat(char *str, char *add)                                                                                                                                                                                                         
{                                                                                                                                                                                                                                             
    int slen = strlen(str), alen = strlen(add);                                                                                                                                                                                               
    char *ret = malloc(sizeof(char) * slen + alen + 1);                                                                                                                                                                                       
    if (ret) {                                                                                                                                                                                                                                
        snprintf(ret, slen + alen, "%s", str);                                                                                                                                                                                                
        snprintf(&ret[slen], alen, "%s", add);                  
        // Free the original str so there is no memory leak                                                                                                                                                                              
        free(str);                                                                                                                                                                                                                            
    }                                                                                                                                                                                                                                         
    return ret;                                                                                                                                                                                                                               
}

Note that my indexes may be off, but this is a simple example showing the issue. I just want to be able to tell sonar, “don’t worry about memory leaks from this function you can’t see, I promise it cleans up after itself”

sonar-cfamily.reproducer (262.5 KB)

14

@KaibutsuX,

I debugged the issue. Two things:

  • For the implementation that you are providing for my_strcat the potential memory leak is there if we combine it with the provided main function. There is no false-positive: if the if (ret) branch is not taken memory will be leaked. If you free str on both branches or you return str when ret is a nullptr the issue should disappear.

  • Now for the reproducer case, the reason for the issue is different since my_strcat definition isn’t available in the file. The issue is raised here because of the __attribute__((malloc)) that you are giving to my_strcat. It has no relation with the name of the function or its implementation. This attribute tells us that we can treat this function like malloc, so we don’t expect a malloc-like function to free memory. In your case, my_strcat doesn’t behave like malloc: it deallocates str, and returns a pointer to valid memory. Removing the malloc attribute from your function declaration should fix the issue.

Thanks,

2 Likes
15

Abbas, thanks for looking into the issue

  1. You’re right, this was a trivial example, but I’ve fixed the implementation of my_strcat. str is now freed regardless before returning ret.

  2. Please see new attached reproducer. False positive still persists without the malloc function attribute.

  • Do you offer any public documentation on how these rules are implemented? If certain function attributes can control the way these rules are interpreted I would love to see all configurations.

sonar-cfamily.reproducer (262.5 KB)

16

@KaibutsuX,

  • Thanks for the new reproducer. I confirm it is a false-positive.
    I created this ticket to allocate the proper time later to investigate it further. Feel free to watch the ticket.

  • No there is no public documentation. These types of rules are always evolving and the aim is to implement them in a way that makes sense so the user shouldn’t be bothered by their internals.
    Which obviously didn’t work out well in your case.

Thanks for the valuable feedback!

17

@Abbas thanks for creating the ticket.

In the meantime aside from ignoring each instance with a block ignore, does sonar offer any method of ignoring specific issues from certain function names?

I would love something to ignore S123:my_strcat

18

@KaibutsuX, unfortunately, no there is nothing like this. You can mark them as false-positive on the SonarQube UI.