GCC10 static analyzer warnings #3377

reinerh · 2020-04-22T17:48:02Z

GCC10 has a new static analyzer (-fanalyzer), that I just tested with the firejail source. (building with CFLAGS=-fanalyzer ./configure && make).
Here are the findings:

gcc -fanalyzer -ggdb  -O2 -DVERSION='"0.9.63"'  -DPREFIX='"/usr/local"' -DSYSCONFDIR='"/usr/local/etc/firejail"' -DLIBDIR='"/usr/local/lib"' -DBINDIR='"/usr/local/bin"' -DHAVE_X11 -DHAVE_PRIVATE_HOME  -DHAVE_OVERLAYFS -DHAVE_FIRETUNNEL -DHAVE_SECCOMP -DHAVE_GLOBALCFG -DHAVE_SECCOMP_H -DHAVE_CHROOT -DHAVE_NETWORK -DHAVE_USERNS -DHAVE_FILE_TRANSFER -DHAVE_WHITELIST  -fstack-protector-all -D_FORTIFY_SOURCE=2 -fPIE -pie -Wformat -Wformat-security -mindirect-branch=thunk -fstack-clash-protection -fstack-protector-strong  -c sbox.c -o sbox.o
sbox.c: In function ‘sbox_run_v’:
sbox.c:288:3: warning: dereference of possibly-NULL ‘arg’ [CWE-690] [-Wanalyzer-possible-null-dereference]
  288 |   fprintf(stderr, "Error: failed to run %s\n", arg[0]);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  ‘sbox_run’: events 1-5
    |
    |  241 | int sbox_run(unsigned filtermask, int num, ...) {
    |      |     ^~~~~~~~
    |      |     |
    |      |     (1) entry to ‘sbox_run’
    |  242 |  va_list valist;
    |      |  ~~~~~~~
    |      |  |
    |      |  (2) this call could return NULL
    |......
    |  248 |  for (i = 0; i < num; i++)
    |      |  ~~~ 
    |      |  |
    |      |  (3) following ‘false’ branch (when ‘i >= num’)...
    |  249 |   arg[i] = va_arg(valist, char *);
    |  250 |  arg[i] = NULL;
    |      |  ~~~ 
    |      |  |
    |      |  (4) ...to here
    |......
    |  253 |  int status = sbox_run_v(filtermask, arg);
    |      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |               |
    |      |               (5) calling ‘sbox_run_v’ from ‘sbox_run’
    |
    +--> ‘sbox_run_v’: events 6-8
           |
           |  260 | int sbox_run_v(unsigned filtermask, char * const arg[]) {
           |      |     ^~~~~~~~~~
           |      |     |
           |      |     (6) entry to ‘sbox_run_v’
           |......
           |  263 |  if (arg_debug) {
           |      |     ~
           |      |     |
           |      |     (7) following ‘false’ branch...
           |......
           |  270 |   printf("\n");
           |      |   ~~~~~~~~~~~~
           |      |   |
           |      |   (8) ...to here
           |
         ‘sbox_run_v’: event 9
           |
           |  274 |  assert((filtermask & SBOX_KEEP_FDS) == 0);
           |      |  ^~~~~~
           |      |  |
           |      |  (9) following ‘true’ branch...
           |
         ‘sbox_run_v’: events 10-11
           |
           |  276 |  pid_t child = fork();
           |      |  ^~~~~
           |      |  |
           |      |  (10) ...to here
           |  277 |  if (child < 0)
           |      |     ~
           |      |     |
           |      |     (11) following ‘false’ branch (when ‘child >= 0’)...
           |
         ‘sbox_run_v’: event 12
           |
           |../include/common.h:35:164:
           |   35 | #define errExit(msg)    do { char msgout[500]; snprintf(msgout, 500, "Error %s: %s:%d %s", msg, __FILE__, __LINE__, __FUNCTION__); perror(msgout); exit(1);} while (0)
           |      |                                                                                                                                                                    ^
           |      |                                                                                                                                                                    |
           |      |                                                                                                                                                                    (12) ...to here
sbox.c:278:3: note: in expansion of macro ‘errExit’
           |  278 |   errExit("fork");
           |      |   ^~~~~~~
           |
         ‘sbox_run_v’: events 13-15
           |
           |  279 |  if (child == 0) {
           |      |     ^
           |      |     |
           |      |     (13) following ‘false’ branch (when ‘child != 0’)...
           |......
           |  283 |  int status;
           |      |  ~~~ 
           |      |  |
           |      |  (14) ...to here
           |  284 |  if (waitpid(child, &status, 0) == -1 ) {
           |      |     ~
           |      |     |
           |      |     (15) following ‘false’ branch...
           |
         ‘sbox_run_v’: event 16
           |
           |../include/common.h:35:164:
           |   35 | #define errExit(msg)    do { char msgout[500]; snprintf(msgout, 500, "Error %s: %s:%d %s", msg, __FILE__, __LINE__, __FUNCTION__); perror(msgout); exit(1);} while (0)
           |      |                                                                                                                                                                    ^
           |      |                                                                                                                                                                    |
           |      |                                                                                                                                                                    (16) ...to here
sbox.c:285:3: note: in expansion of macro ‘errExit’
           |  285 |   errExit("waitpid");
           |      |   ^~~~~~~
           |
         ‘sbox_run_v’: event 17
           |
           |  287 |  if (WIFEXITED(status) && status != 0) {
           |      |     ^
           |      |     |
           |      |     (17) following ‘true’ branch...
           |
         ‘sbox_run_v’: event 18
           |
           |  287 |  if (WIFEXITED(status) && status != 0) {
           |      |                        ^
           |      |                        |
           |      |                        (18) ...to here
           |
         ‘sbox_run_v’: events 19-21
           |
           |

gcc -fanalyzer -ggdb  -O2 -DVERSION='"0.9.63"'  -DPREFIX='"/usr/local"' -DSYSCONFDIR='"/usr/local/etc/firejail"' -DLIBDIR='"/usr/local/lib"' -DBINDIR='"/usr/local/bin"' -DHAVE_X11 -DHAVE_PRIVATE_HOME  -DHAVE_OVERLAYFS -DHAVE_FIRETUNNEL -DHAVE_SECCOMP -DHAVE_GLOBALCFG -DHAVE_SECCOMP_H -DHAVE_CHROOT -DHAVE_NETWORK -DHAVE_USERNS -DHAVE_FILE_TRANSFER -DHAVE_WHITELIST  -fstack-protector-all -D_FORTIFY_SOURCE=2 -fPIE -pie -Wformat -Wformat-security -mindirect-branch=thunk -fstack-clash-protection -fstack-protector-strong  -c paths.c -o paths.o
paths.c: In function ‘init_paths’:
paths.c:79:1: warning: leak of ‘<unknown>’ [CWE-401] [-Wanalyzer-malloc-leak]
   79 | }
      | ^
  ‘count_paths’: events 1-4
    |
    |   90 | unsigned int count_paths(void) {
    |      |              ^~~~~~~~~~~
    |      |              |
    |      |              (1) entry to ‘count_paths’
    |   91 |  if (!path_cnt)
    |      |     ~         
    |      |     |
    |      |     (2) following ‘true’ branch...
    |   92 |   init_paths();
    |      |   ~~~~~~~~~~~~
    |      |   |
    |      |   (3) ...to here
    |      |   (4) calling ‘init_paths’ from ‘count_paths’
    |
    +--> ‘init_paths’: events 5-6
           |
           |   28 | static void init_paths(void) {
           |      |             ^~~~~~~~~~
           |      |             |
           |      |             (5) entry to ‘init_paths’
           |......
           |   36 |  if (!path)
           |      |     ~        
           |      |     |
           |      |     (6) following ‘false’ branch...
           |
         ‘init_paths’: event 7
           |
           |cc1:
           | (7): ...to here
           |
         ‘init_paths’: events 8-12
           |
           |   40 |  for (p = path; *p; p++)
           |      |  ^~~
           |      |  |
           |      |  (8) following ‘false’ branch...
           |......
           |   43 |  path_cnt += 2; // one because we were counting fenceposts, one for the NULL at the end
           |      |  ~~~~~~~~
           |      |  |
           |      |  (9) ...to here
           |      |  (10) allocated here
           |......
           |   46 |  if (!paths)
           |      |     ~
           |      |     |
           |      |     (11) assuming ‘<unknown>’ is non-NULL
           |      |     (12) following ‘false’ branch...
           |
         ‘init_paths’: event 13
           |
           |cc1:
           | (13): ...to here
           |
         ‘init_paths’: event 14
           |
           |   52 |  while ((elt = strsep(&path, ":")) != NULL) {
           |      |        ^
           |      |        |
           |      |        (14) following ‘false’ branch...
           |
         ‘init_paths’: event 15
           |
           |   76 |  assert(paths[i] == NULL);
           |      |  ^~~~~~
           |      |  |
           |      |  (15) ...to here
           |
         ‘init_paths’: event 16
           |
           |   76 |  assert(paths[i] == NULL);
           |      |  ^~~~~~
           |      |  |
           |      |  (16) following ‘true’ branch...
           |
         ‘init_paths’: events 17-18
           |
           |   78 |  path_cnt = i+1;
           |      |  ^~~~~~~~
           |      |  |
           |      |  (17) ...to here
           |   79 | }
           |      | ~ 
           |      | |
           |      | (18) ‘<unknown>’ leaks here; was allocated at (10)
           |

looks like false positive to me:

gcc -fanalyzer -ggdb  -O2 -DVERSION='"0.9.63"'  -DPREFIX='"/usr/local"' -DSYSCONFDIR='"/usr/local/etc/firejail"' -DLIBDIR='"/usr/local/lib"' -DBINDIR='"/usr/local/bin"' -DHAVE_X11 -DHAVE_PRIVATE_HOME  -DHAVE_OVERLAYFS -DHAVE_FIRETUNNEL -DHAVE_SECCOMP -DHAVE_GLOBALCFG -DHAVE_SECCOMP_H -DHAVE_CHROOT -DHAVE_NETWORK -DHAVE_USERNS -DHAVE_FILE_TRANSFER -DHAVE_WHITELIST  -fstack-protector-all -D_FORTIFY_SOURCE=2 -fPIE -pie -Wformat -Wformat-security -mindirect-branch=thunk -fstack-clash-protection -fstack-protector-strong  -c ls.c -o ls.o
In function ‘print_directory’:
ls.c:182:4: warning: double-‘free’ of ‘<unknown>’ [CWE-415] [-Wanalyzer-double-free]
  182 |    free(namelist[i]);
      |    ^~~~~~~~~~~~~~~~~
  ‘sandboxfs’: event 1
    |
    |  211 | void sandboxfs(int op, pid_t pid, const char *path1, const char *path2) {
    |      |      ^~~~~~~~~
    |      |      |
    |      |      (1) entry to ‘sandboxfs’
    |
  ‘sandboxfs’: event 2
    |
    |  213 |  assert(path1);
    |      |  ^~~~~~
    |      |  |
    |      |  (2) following ‘true’ branch (when ‘path1’ is non-NULL)...
    |
  ‘sandboxfs’: events 3-4
    |
    |  216 |  pid = switch_to_child(pid);
    |      |  ^~~
    |      |  |
    |      |  (3) ...to here
    |......
    |  222 |  char *fname1 = expand_path(path1);;
    |      |                 ~~~~~~~~~~~~~~~~~~
    |      |                 |
    |      |                 (4) calling ‘expand_path’ from ‘sandboxfs’
    |
    +--> ‘expand_path’: events 5-6
           |
           |  188 | char *expand_path(const char *path) {
           |      |       ^~~~~~~~~~~
           |      |       |
           |      |       (5) entry to ‘expand_path’
           |......
           |  192 |   if (!fname)
           |      |      ~ 
           |      |      |
           |      |      (6) following ‘false’ branch...
           |
         ‘expand_path’: event 7
           |
           |../include/common.h:35:164:
           |   35 | #define errExit(msg)    do { char msgout[500]; snprintf(msgout, 500, "Error %s: %s:%d %s", msg, __FILE__, __LINE__, __FUNCTION__); perror(msgout); exit(1);} while (0)
           |      |                                                                                                                                                                    ^
           |      |                                                                                                                                                                    |
           |      |                                                                                                                                                                    (7) ...to here
ls.c:206:4: note: in expansion of macro ‘errExit’
           |  206 |    errExit("asprintf");
           |      |    ^~~~~~~
           |
    <------+
    |
  ‘sandboxfs’: events 8-9
    |
    |  222 |  char *fname1 = expand_path(path1);;
    |      |                 ^~~~~~~~~~~~~~~~~~
    |      |                 |
    |      |                 (8) returning to ‘sandboxfs’ from ‘expand_path’
    |......
    |  234 |  if (asprintf(&rootdir, "/proc/%d/root", pid) == -1)
    |      |     ~            
    |      |     |
    |      |     (9) following ‘false’ branch...
    |
  ‘sandboxfs’: event 10
    |
    |../include/common.h:35:164:
    |   35 | #define errExit(msg)    do { char msgout[500]; snprintf(msgout, 500, "Error %s: %s:%d %s", msg, __FILE__, __LINE__, __FUNCTION__); perror(msgout); exit(1);} while (0)
    |      |                                                                                                                                                                    ^
    |      |                                                                                                                                                                    |
    |      |                                                                                                                                                                    (10) ...to here
ls.c:235:3: note: in expansion of macro ‘errExit’
    |  235 |   errExit("asprintf");
    |      |   ^~~~~~~
    |
  ‘sandboxfs’: events 11-13
    |
    |  237 |  if (op == SANDBOX_FS_LS) {
    |      |     ^
    |      |     |
    |      |     (11) following ‘true’ branch (when ‘op == 0’)...
    |  238 |   EUID_ROOT();
    |      |   ~~~~~~~~~
    |      |   |
    |      |   (12) ...to here
    |  239 |   // chroot
    |  240 |   if (chroot(rootdir) < 0)
    |      |      ~
    |      |      |
    |      |      (13) following ‘false’ branch...
    |
  ‘sandboxfs’: event 14
    |
    |../include/common.h:35:164:
    |   35 | #define errExit(msg)    do { char msgout[500]; snprintf(msgout, 500, "Error %s: %s:%d %s", msg, __FILE__, __LINE__, __FUNCTION__); perror(msgout); exit(1);} while (0)
    |      |                                                                                                                                                                    ^
    |      |                                                                                                                                                                    |
    |      |                                                                                                                                                                    (14) ...to here
ls.c:241:4: note: in expansion of macro ‘errExit’
    |  241 |    errExit("chroot");
    |      |    ^~~~~~~
    |
  ‘sandboxfs’: event 15
    |
    |  242 |   if (chdir("/") < 0)
    |      |      ^
    |      |      |
    |      |      (15) following ‘false’ branch...
    |
  ‘sandboxfs’: event 16
    |
    |../include/common.h:35:164:
    |   35 | #define errExit(msg)    do { char msgout[500]; snprintf(msgout, 500, "Error %s: %s:%d %s", msg, __FILE__, __LINE__, __FUNCTION__); perror(msgout); exit(1);} while (0)
    |      |                                                                                                                                                                    ^
    |      |                                                                                                                                                                    |
    |      |                                                                                                                                                                    (16) ...to here
ls.c:243:4: note: in expansion of macro ‘errExit’
    |  243 |    errExit("chdir");
    |      |    ^~~~~~~
    |
  ‘sandboxfs’: events 17-25
    |
    |  249 |   if (access(fname1, R_OK) == -1) {
    |      |      ^
    |      |      |
    |      |      (17) following ‘false’ branch...
    |......
    |  254 |   char *rp = realpath(fname1, NULL);
    |      |   ~~~~
    |      |   |
    |      |   (18) ...to here
    |  255 |   if (!rp) {
    |      |      ~
    |      |      |
    |      |      (19) following ‘false’ branch...
    |......
    |  259 |   if (arg_debug)
    |      |   ~~  
    |      |   |
    |      |   (20) ...to here
    |......
    |  265 |   if (stat(rp, &s) == -1) {
    |      |      ~
    |      |      |
    |      |      (21) following ‘false’ branch...
    |......
    |  269 |   if (S_ISDIR(s.st_mode)) {
    |      |   ~~ ~
    |      |   |  |
    |      |   |  (23) following ‘true’ branch...
    |      |   (22) ...to here
    |  270 |    char *dir;
    |      |    ~~~~
    |      |    |
    |      |    (24) ...to here
    |  271 |    if (asprintf(&dir, "%s/", rp) == -1)
    |      |       ~
    |      |       |
    |      |       (25) following ‘false’ branch...
    |
  ‘sandboxfs’: event 26
    |
    |../include/common.h:35:164:
    |   35 | #define errExit(msg)    do { char msgout[500]; snprintf(msgout, 500, "Error %s: %s:%d %s", msg, __FILE__, __LINE__, __FUNCTION__); perror(msgout); exit(1);} while (0)
    |      |                                                                                                                                                                    ^
    |      |                                                                                                                                                                    |
    |      |                                                                                                                                                                    (26) ...to here
ls.c:272:5: note: in expansion of macro ‘errExit’
    |  272 |     errExit("asprintf");
    |      |     ^~~~~~~
    |
  ‘sandboxfs’: event 27
    |
    |  274 |    print_directory(dir);
    |      |    ^~~~~~~~~~~~~~~~~~~~
    |      |    |
    |      |    (27) calling ‘print_directory’ from ‘sandboxfs’
    |
    +--> ‘print_directory’: event 28
           |
           |  165 | static void print_directory(const char *path) {
           |      |             ^~~~~~~~~~~~~~~
           |      |             |
           |      |             (28) entry to ‘print_directory’
           |
         ‘print_directory’: event 29
           |
           |  166 |  assert(path);
           |      |  ^~~~~~
           |      |  |
           |      |  (29) following ‘true’ branch (when ‘path’ is non-NULL)...
           |
         ‘print_directory’: events 30-31
           |
           |  167 |  struct stat s;
           |      |  ^~~~~~
           |      |  |
           |      |  (30) ...to here
           |  168 |  if (stat(path, &s) == -1)
           |      |     ~
           |      |     |
           |      |     (31) following ‘false’ branch...
           |
         ‘print_directory’: event 32
           |
           |  170 |  assert(S_ISDIR(s.st_mode));
           |      |  ^~~~~~
           |      |  |
           |      |  (32) ...to here
           |
         ‘print_directory’: event 33
           |
           |  170 |  assert(S_ISDIR(s.st_mode));
           |      |  ^~~~~~
           |      |  |
           |      |  (33) following ‘true’ branch...
           |
         ‘print_directory’: events 34-35
           |
           |  172 |  struct dirent **namelist;
           |      |  ^~~~~~
           |      |  |
           |      |  (34) ...to here
           |......
           |  177 |  if (n < 0)
           |      |     ~
           |      |     |
           |      |     (35) following ‘false’ branch (when ‘n >= 0’)...
           |
         ‘print_directory’: event 36
           |
           |cc1:
           | (36): ...to here
           |
         ‘print_directory’: events 37-39
           |
           |  180 |   for (i = 0; i < n; i++) {
           |      |   ^~~
           |      |   |
           |      |   (37) following ‘true’ branch (when ‘i < n’)...
           |  181 |    print_file_or_dir(path, namelist[i]->d_name, 0);
           |      |    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |    |
           |      |    (38) ...to here
           |      |    (39) calling ‘print_file_or_dir’ from ‘print_directory’
           |
           +--> ‘print_file_or_dir’: event 40
                  |
                  |   37 | static void print_file_or_dir(const char *path, const char *fname, int separator) {
                  |      |             ^~~~~~~~~~~~~~~~~
                  |      |             |
                  |      |             (40) entry to ‘print_file_or_dir’
                  |
                ‘print_file_or_dir’: event 41
                  |
                  |   38 |  assert(fname);
                  |      |  ^~~~~~
                  |      |  |
                  |      |  (41) following ‘true’ branch (when ‘fname’ is non-NULL)...
                  |
                ‘print_file_or_dir’: events 42-45
                  |
                  |   40 |  char *name;
                  |      |  ^~~~
                  |      |  |
                  |      |  (42) ...to here
                  |   41 |  if (separator) {
                  |      |     ~
                  |      |     |
                  |      |     (43) following ‘false’ branch (when ‘separator == 0’)...
                  |......
                  |   46 |   if (asprintf(&name, "%s%s", path, fname) == -1)
                  |      |   ~~ ~
                  |      |   |  |
                  |      |   |  (45) following ‘false’ branch...
                  |      |   (44) ...to here
                  |
                ‘print_file_or_dir’: event 46
                  |
                  |../include/common.h:35:164:
                  |   35 | #define errExit(msg)    do { char msgout[500]; snprintf(msgout, 500, "Error %s: %s:%d %s", msg, __FILE__, __LINE__, __FUNCTION__); perror(msgout); exit(1);} while (0)
                  |      |                                                                                                                                                                    ^
                  |      |                                                                                                                                                                    |
                  |      |                                                                                                                                                                    (46) ...to here
ls.c:47:4: note: in expansion of macro ‘errExit’
                  |   47 |    errExit("asprintf");
                  |      |    ^~~~~~~
                  |
                ‘print_file_or_dir’: events 47-50
                  |
                  |   51 |  if (stat(name, &s) == -1) {
                  |      |     ^
                  |      |     |
                  |      |     (47) following ‘true’ branch...
                  |   52 |   if (lstat(name, &s) == -1) {
                  |      |   ~~ ~
                  |      |   |  |
                  |      |   |  (49) following ‘true’ branch...
                  |      |   (48) ...to here
                  |   53 |    printf("Error: cannot access %s\n", name);
                  |      |    ~~~~~~
                  |      |    |
                  |      |    (50) ...to here
                  |
           <------+
           |
         ‘print_directory’: events 51-55
           |
           |  180 |   for (i = 0; i < n; i++) {
           |      |   ~~~
           |      |   |
           |      |   (53) following ‘true’ branch (when ‘i < n’)...
           |  181 |    print_file_or_dir(path, namelist[i]->d_name, 0);
           |      |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |    |
           |      |    (51) returning to ‘print_directory’ from ‘print_file_or_dir’
           |      |    (54) ...to here
           |      |    (55) calling ‘print_file_or_dir’ from ‘print_directory’
           |  182 |    free(namelist[i]);
           |      |    ~~~~
           |      |    |
           |      |    (52) first ‘free’ here
           |
           +--> ‘print_file_or_dir’: event 56
                  |
                  |   37 | static void print_file_or_dir(const char *path, const char *fname, int separator) {
                  |      |             ^~~~~~~~~~~~~~~~~
                  |      |             |
                  |      |             (56) entry to ‘print_file_or_dir’
                  |
                ‘print_file_or_dir’: event 57
                  |
                  |   38 |  assert(fname);
                  |      |  ^~~~~~
                  |      |  |
                  |      |  (57) following ‘true’ branch (when ‘fname’ is non-NULL)...
                  |
                ‘print_file_or_dir’: events 58-61
                  |
                  |   40 |  char *name;
                  |      |  ^~~~
                  |      |  |
                  |      |  (58) ...to here
                  |   41 |  if (separator) {
                  |      |     ~
                  |      |     |
                  |      |     (59) following ‘false’ branch (when ‘separator == 0’)...
                  |......
                  |   46 |   if (asprintf(&name, "%s%s", path, fname) == -1)
                  |      |   ~~ ~
                  |      |   |  |
                  |      |   |  (61) following ‘false’ branch...
                  |      |   (60) ...to here
                  |
                ‘print_file_or_dir’: event 62
                  |
                  |../include/common.h:35:164:
                  |   35 | #define errExit(msg)    do { char msgout[500]; snprintf(msgout, 500, "Error %s: %s:%d %s", msg, __FILE__, __LINE__, __FUNCTION__); perror(msgout); exit(1);} while (0)
                  |      |                                                                                                                                                                    ^
                  |      |                                                                                                                                                                    |
                  |      |                                                                                                                                                                    (62) ...to here
ls.c:47:4: note: in expansion of macro ‘errExit’
                  |   47 |    errExit("asprintf");
                  |      |    ^~~~~~~
                  |
                ‘print_file_or_dir’: events 63-66
                  |
                  |   51 |  if (stat(name, &s) == -1) {
                  |      |     ^
                  |      |     |
                  |      |     (63) following ‘true’ branch...
                  |   52 |   if (lstat(name, &s) == -1) {
                  |      |   ~~ ~
                  |      |   |  |
                  |      |   |  (65) following ‘true’ branch...
                  |      |   (64) ...to here
                  |   53 |    printf("Error: cannot access %s\n", name);
                  |      |    ~~~~~~
                  |      |    |
                  |      |    (66) ...to here
                  |
           <------+
           |
         ‘print_directory’: events 67-71
           |
           |  180 |   for (i = 0; i < n; i++) {
           |      |   ~~~
           |      |   |
           |      |   (69) following ‘true’ branch (when ‘i < n’)...
           |  181 |    print_file_or_dir(path, namelist[i]->d_name, 0);
           |      |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |    |
           |      |    (67) returning to ‘print_directory’ from ‘print_file_or_dir’
           |      |    (70) ...to here
           |      |    (71) calling ‘print_file_or_dir’ from ‘print_directory’
           |  182 |    free(namelist[i]);
           |      |    ~~~~
           |      |    |
           |      |    (68) first ‘free’ here
           |
           +--> ‘print_file_or_dir’: event 72
                  |
                  |   37 | static void print_file_or_dir(const char *path, const char *fname, int separator) {
                  |      |             ^~~~~~~~~~~~~~~~~
                  |      |             |
                  |      |             (72) entry to ‘print_file_or_dir’
                  |
                ‘print_file_or_dir’: event 73
                  |
                  |   38 |  assert(fname);
                  |      |  ^~~~~~
                  |      |  |
                  |      |  (73) following ‘true’ branch (when ‘fname’ is non-NULL)...
                  |
                ‘print_file_or_dir’: events 74-77
                  |
                  |   40 |  char *name;
                  |      |  ^~~~
                  |      |  |
                  |      |  (74) ...to here
                  |   41 |  if (separator) {
                  |      |     ~
                  |      |     |
                  |      |     (75) following ‘false’ branch (when ‘separator == 0’)...
                  |......
                  |   46 |   if (asprintf(&name, "%s%s", path, fname) == -1)
                  |      |   ~~ ~
                  |      |   |  |
                  |      |   |  (77) following ‘false’ branch...
                  |      |   (76) ...to here
                  |
                ‘print_file_or_dir’: event 78
                  |
                  |../include/common.h:35:164:
                  |   35 | #define errExit(msg)    do { char msgout[500]; snprintf(msgout, 500, "Error %s: %s:%d %s", msg, __FILE__, __LINE__, __FUNCTION__); perror(msgout); exit(1);} while (0)
                  |      |                                                                                                                                                                    ^
                  |      |                                                                                                                                                                    |
                  |      |                                                                                                                                                                    (78) ...to here
ls.c:47:4: note: in expansion of macro ‘errExit’
                  |   47 |    errExit("asprintf");
                  |      |    ^~~~~~~
                  |
                ‘print_file_or_dir’: events 79-82
                  |
                  |   51 |  if (stat(name, &s) == -1) {
                  |      |     ^
                  |      |     |
                  |      |     (79) following ‘true’ branch...
                  |   52 |   if (lstat(name, &s) == -1) {
                  |      |   ~~ ~
                  |      |   |  |
                  |      |   |  (81) following ‘true’ branch...
                  |      |   (80) ...to here
                  |   53 |    printf("Error: cannot access %s\n", name);
                  |      |    ~~~~~~
                  |      |    |
                  |      |    (82) ...to here
                  |
           <------+
           |
         ‘print_directory’: events 83-84
           |
           |  181 |    print_file_or_dir(path, namelist[i]->d_name, 0);
           |      |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |    |
           |      |    (83) returning to ‘print_directory’ from ‘print_file_or_dir’
           |  182 |    free(namelist[i]);
           |      |    ~~~~~~~~~~~~~~~~~
           |      |    |
           |      |    (84) second ‘free’ here; first ‘free’ was at (68)
           |

Fixed in c02c91e:

gcc -fanalyzer -ggdb  -O2 -DVERSION='"0.9.63"'  -DPREFIX='"/usr/local"' -DSYSCONFDIR='"/usr/local/etc/firejail"' -DLIBDIR='"/usr/local/lib"' -DBINDIR='"/usr/local/bin"' -DHAVE_X11 -DHAVE_PRIVATE_HOME  -DHAVE_OVERLAYFS -DHAVE_FIRETUNNEL -DHAVE_SECCOMP -DHAVE_GLOBALCFG -DHAVE_SECCOMP_H -DHAVE_CHROOT -DHAVE_NETWORK -DHAVE_USERNS -DHAVE_FILE_TRANSFER -DHAVE_WHITELIST  -fstack-protector-all -D_FORTIFY_SOURCE=2 -fPIE -pie -Wformat -Wformat-security -mindirect-branch=thunk -fstack-clash-protection -fstack-protector-strong  -c pid.c -o pid.o
pid.c: In function ‘pid_test’:
pid.c:70:8: warning: use of uninitialized value ‘<unknown>’ [CWE-457] [-Wanalyzer-use-of-uninitialized-value]
   70 |    if (strncmp(buf, kern_proc[j], strlen(kern_proc[j])) == 0) {
      |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  ‘pid_test’: events 1-3
    |
    |   35 |  for (i = 1; i <= 10; i++) {
    |      |  ^~~
    |      |  |
    |      |  (1) following ‘true’ branch (when ‘i != 11’)...
    |   36 |   struct stat s;
    |      |   ~~~~~~
    |      |   |
    |      |   (2) ...to here
    |   37 |   char *fname;
    |   38 |   if (asprintf(&fname, "/proc/%d/comm", i) == -1)
    |      |      ~
    |      |      |
    |      |      (3) following ‘false’ branch...
    |
  ‘pid_test’: event 4
    |
    |faudit.h:34:151:
    |   34 | #define errExit(msg)    do { char msgout[500]; snprintf(msgout, 500, "Error %s:%s(%d)", msg, __FUNCTION__, __LINE__); perror(msgout); exit(1);} while (0)
    |      |                                                                                                                                                       ^
    |      |                                                                                                                                                       |
    |      |                                                                                                                                                       (4) ...to here
pid.c:39:4: note: in expansion of macro ‘errExit’
    |   39 |    errExit("asprintf");
    |      |    ^~~~~~~
    |
  ‘pid_test’: events 5-19
    |
    |   40 |   if (stat(fname, &s) == -1) {
    |      |      ^
    |      |      |
    |      |      (5) following ‘false’ branch...
    |......
    |   47 |   FILE *fp = fopen(fname, "r");
    |      |   ~~~~
    |      |   |
    |      |   (6) ...to here
    |   48 |   if (!fp) {
    |      |      ~
    |      |      |
    |      |      (7) following ‘false’ branch (when ‘fp’ is non-NULL)...
    |......
    |   54 |   char buf[100];
    |      |   ~~~~
    |      |   |
    |      |   (8) ...to here
    |   55 |   if (fgets(buf, 10, fp) == NULL) {
    |      |      ~
    |      |      |
    |      |      (9) following ‘false’ branch...
    |......
    |   60 |   not_visible = 0;
    |      |   ~~~~~~~~~~~
    |      |   |
    |      |   (10) ...to here
    |......
    |   64 |   if ((ptr = strchr(buf, '\n')) != NULL)
    |      |      ~
    |      |      |
    |      |      (11) following ‘false’ branch (when ‘ptr’ is NULL)...
    |......
    |   68 |   int j = 0;
    |      |   ~~~ 
    |      |   |
    |      |   (12) ...to here
    |   69 |   while (kern_proc[j] != NULL) {
    |      |         ~
    |      |         |
    |      |         (13) following ‘true’ branch...
    |      |         (17) following ‘true’ branch...
    |   70 |    if (strncmp(buf, kern_proc[j], strlen(kern_proc[j])) == 0) {
    |      |    ~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |    |  ||
    |      |    |  |(19) use of uninitialized value ‘<unknown>’ here
    |      |    |  (15) following ‘false’ branch...
    |      |    (14) ...to here
    |      |    (18) ...to here
    |......
    |   77 |    j++;
    |      |    ~  
    |      |    |
    |      |    (16) ...to here
    |

Fixed in 2d16063:

gcc -fanalyzer -ggdb  -O2 -DVERSION='"0.9.63"'  -DPREFIX='"/usr/local"' -DSYSCONFDIR='"/usr/local/etc/firejail"' -DLIBDIR='"/usr/local/lib"' -DBINDIR='"/usr/local/bin"' -DHAVE_X11 -DHAVE_PRIVATE_HOME  -DHAVE_OVERLAYFS -DHAVE_FIRETUNNEL -DHAVE_SECCOMP -DHAVE_GLOBALCFG -DHAVE_SECCOMP_H -DHAVE_CHROOT -DHAVE_NETWORK -DHAVE_USERNS -DHAVE_FILE_TRANSFER -DHAVE_WHITELIST  -fstack-protector-all -D_FORTIFY_SOURCE=2 -fPIE -pie -Wformat -Wformat-security -mindirect-branch=thunk -fstack-clash-protection -fstack-protector-strong  -c no_sandbox.c -o no_sandbox.o
no_sandbox.c: In function ‘check_kernel_procs’:
no_sandbox.c:142:8: warning: use of uninitialized value ‘<unknown>’ [CWE-457] [-Wanalyzer-use-of-uninitialized-value]
  142 |    if (strncmp(buf, kern_proc[j], strlen(kern_proc[j])) == 0) {
      |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  ‘check_kernel_procs’: events 1-5
    |
    |  102 |  if (arg_debug)
    |      |     ^
    |      |     |
    |      |     (1) following ‘true’ branch...
    |  103 |   printf("Looking for kernel processes\n");
    |      |   ~~~~~~
    |      |   |
    |      |   (2) ...to here
    |......
    |  107 |  for (i = 1; i <= 10; i++) {
    |      |  ~~~ 
    |      |  |
    |      |  (3) following ‘true’ branch (when ‘i != 11’)...
    |  108 |   struct stat s;
    |      |   ~~~~~~
    |      |   |
    |      |   (4) ...to here
    |  109 |   char *fname;
    |  110 |   if (asprintf(&fname, "/proc/%d/comm", i) == -1)
    |      |      ~
    |      |      |
    |      |      (5) following ‘false’ branch...
    |
  ‘check_kernel_procs’: event 6
    |
    |../include/common.h:35:164:
    |   35 | #define errExit(msg)    do { char msgout[500]; snprintf(msgout, 500, "Error %s: %s:%d %s", msg, __FILE__, __LINE__, __FUNCTION__); perror(msgout); exit(1);} while (0)
    |      |                                                                                                                                                                    ^
    |      |                                                                                                                                                                    |
    |      |                                                                                                                                                                    (6) ...to here
no_sandbox.c:111:4: note: in expansion of macro ‘errExit’
    |  111 |    errExit("asprintf");
    |      |    ^~~~~~~
    |
  ‘check_kernel_procs’: events 7-21
    |
    |  112 |   if (stat(fname, &s) == -1) {
    |      |      ^
    |      |      |
    |      |      (7) following ‘false’ branch...
    |......
    |  119 |   FILE *fp = fopen(fname, "r");
    |      |   ~~~~
    |      |   |
    |      |   (8) ...to here
    |  120 |   if (!fp) {
    |      |      ~
    |      |      |
    |      |      (9) following ‘false’ branch (when ‘fp’ is non-NULL)...
    |......
    |  127 |   char buf[100];
    |      |   ~~~~
    |      |   |
    |      |   (10) ...to here
    |  128 |   if (fgets(buf, 10, fp) == NULL) {
    |      |      ~
    |      |      |
    |      |      (11) following ‘false’ branch...
    |......
    |  135 |   char *ptr;
    |      |   ~~~~
    |      |   |
    |      |   (12) ...to here
    |  136 |   if ((ptr = strchr(buf, '\n')) != NULL)
    |      |      ~
    |      |      |
    |      |      (13) following ‘false’ branch (when ‘ptr’ is NULL)...
    |......
    |  140 |   int j = 0;
    |      |   ~~~ 
    |      |   |
    |      |   (14) ...to here
    |  141 |   while (kern_proc[j] != NULL) {
    |      |         ~
    |      |         |
    |      |         (15) following ‘true’ branch...
    |      |         (19) following ‘true’ branch...
    |  142 |    if (strncmp(buf, kern_proc[j], strlen(kern_proc[j])) == 0) {
    |      |    ~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |    |  ||
    |      |    |  |(21) use of uninitialized value ‘<unknown>’ here
    |      |    |  (17) following ‘false’ branch...
    |      |    (16) ...to here
    |      |    (20) ...to here
    |......
    |  149 |    j++;
    |      |    ~  
    |      |    |
    |      |    (18) ...to here
    |

The text was updated successfully, but these errors were encountered:

netblue30 · 2020-10-01T11:46:35Z

All set, tested on Arch! I added support to ./configure:

$ ./configure --prefix=/usr --enable-analyzer

rusty-snake mentioned this issue Sep 1, 2020

Must-fix bugs for release 0.9.64 #3618

Closed

reinerh added this to the 0.9.64 milestone Sep 1, 2020

netblue30 pushed a commit that referenced this issue Sep 28, 2020

clean gcc ananlyzer warnings - #3377

823dd6b

netblue30 closed this as completed Oct 1, 2020

matu3ba mentioned this issue Oct 7, 2021

luarocks matu3ba/firejail#1

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GCC10 static analyzer warnings #3377

GCC10 static analyzer warnings #3377

reinerh commented Apr 22, 2020 •

edited

Loading

netblue30 commented Oct 1, 2020

GCC10 static analyzer warnings #3377

GCC10 static analyzer warnings #3377

Comments

reinerh commented Apr 22, 2020 • edited Loading

netblue30 commented Oct 1, 2020

reinerh commented Apr 22, 2020 •

edited

Loading