Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

--build creates invalid paths with "after,$HOME" $HOME expanded #4592

Open
2 of 7 tasks
matu3ba opened this issue Oct 5, 2021 · 6 comments
Open
2 of 7 tasks

--build creates invalid paths with "after,$HOME" $HOME expanded #4592

matu3ba opened this issue Oct 5, 2021 · 6 comments

Comments

@matu3ba
Copy link
Contributor

matu3ba commented Oct 5, 2021 •
edited
Loading

Description

Path handling for firejail --build nvim is broken.

Steps to Reproduce

Steps to reproduce the behavior

  1. Run in bash LANG=C firejail --build nvim
  2. See result contains stuff like allow ${HOME}/.local/share/nvim/site/after,/etc/xdg/nvim/after,/home/misterspoon/.config/kdedefaults/nvim/after,/home/misterspoon/.config/nvim/after/start
  3. Install from git to have profile lookup.
  4. firejail nvim
  5. Get error shown in actual behavior

Expected behavior

There should be never invalid paths generated. I hope this code is not used elsewhere.

Actual behavior

site/after,/ ... nvim/after, and especially /kdedefaults/nvim/after, looks broken. Invalid path generated.

Error: "${HOME}/.local/share/nvim/site/after,/etc/xdg/nvim/after,/home/misterspoon/.config/kdedefaults/nvim/after,/home/misterspoon/.config/nvim/after/start" is an invalid filename: rejected character: ","

Additional context

PATH contains the path to nvim installation from source.
I have a file $HOME/.local/share/nvim/site/pack/packer/start/coq_nvim/.vars/runtime/lib/python3.9/site-packages/pip/_vendor/tenacity/after.py
and
$HOME/.local/share/nvim/site/pack/packer/start/coq_nvim/.vars/runtime/lib/python3.9/site-packages/pip/_vendor/tenacity/pycache/after.cpython-39.pyc.

Environment

  • Linux distribution and version Linux 5.14 Arch (Endeavour)
  • Firejail version (firejail --version). master with commit 32fb5ed

Checklist

  • The issues is caused by firejail (i.e. running the program by path (e.g. /usr/bin/vlc) "fixes" it).
  • I can reproduce the issue without custom modifications (e.g. globals.local).
  • The program has a profile. (If not, request one in https://github.com/netblue30/firejail/issues/1139)
  • The profile (and redirect profile if exists) hasn't already been fixed upstream.
  • I have performed a short search for similar issues (to avoid opening a duplicate).
  • I'm aware of browser-allow-drm yes/browser-disable-u2f no in firejail.config to allow DRM/U2F in browsers.
  • I used --profile=PROFILENAME to set the right profile. (Only relevant for AppImages)

Log

Output of firejail --build nvim 

# Firejail profile for nvim
# Persistent local customizations
#include nvim.local
# Persistent global definitions
#include globals.local

### Basic Blacklisting ###
### Enable as many of them as you can! A very important one is
### "disable-exec.inc". This will make among other things your home
### and /tmp directories non-executable.
include disable-common.inc	# dangerous directories like ~/.ssh and ~/.gnupg
#include disable-devel.inc	# development tools such as gcc and gdb
#include disable-exec.inc	# non-executable directories such as /var, /tmp, and /home
#include disable-interpreters.inc	# perl, python, lua etc.
include disable-programs.inc	# user configuration for programs such as firefox, vlc etc.
#include disable-shell.inc	# sh, bash, zsh etc.
#include disable-xdg.inc	# standard user directories: Documents, Pictures, Videos, Music

### Home Directory Whitelisting ###
### If something goes wrong, this section is the first one to comment out.
### Instead, you'll have to relay on the basic blacklisting above.
allow ${HOME}/.local/nvim/lib/nvim
allow ${HOME}/.gitconfig
allow ${HOME}/.config/git
allow ${HOME}/.local/nvim/lib/nvim/plugin
allow ${HOME}/.local/share/nvim
allow ${HOME}/.local/share/nvim/site/plugin
allow ${HOME}/.local/share/nvim/site/after/pack
allow ${HOME}/.local/share/nvim/site/after/start
allow ${HOME}/.local/nvim/lib/nvim/ftdetect
allow ${HOME}/.local/share/nvim/site/pack/packer/start/zig.vim/ftdetect
allow ${HOME}/.local/share/nvim/site/pack/packer/start/zig.vim/ftdetect/
allow ${HOME}/.local/share/nvim/site/pack/packer/start/which-key.nvim/ftdetect
allow ${HOME}/.local/share/nvim/site/pack/packer/start/todo-comments.nvim/ftdetect
allow ${HOME}/.local/share/nvim/site/pack/packer/start/telescope.nvim/ftdetect
allow ${HOME}/.local/share/nvim/site/pack/packer/start/telescope-project.nvim/ftdetect
allow ${HOME}/.local/share/nvim/site/pack/packer/start/telescope-fzf-native.nvim/ftdetect
allow ${HOME}/.local/share/nvim/site/pack/packer/start/popup.nvim/ftdetect
allow ${HOME}/.local/share/nvim/site/pack/packer/start/plenary.nvim/ftdetect
allow ${HOME}/.local/share/nvim/site/pack/packer/start/packer.nvim/ftdetect
allow ${HOME}/.local/share/nvim/site/pack/packer/start/one-small-step-for-vimkind/ftdetect
allow ${HOME}/.local/share/nvim/site/pack/packer/start/nvim-luadev/ftdetect
allow ${HOME}/.local/share/nvim/site/pack/packer/start/nvim-lspconfig/ftdetect
allow ${HOME}/.local/share/nvim/site/pack/packer/start/material.nvim/ftdetect
allow ${HOME}/.local/share/nvim/site/pack/packer/start/lightspeed.nvim/ftdetect
allow ${HOME}/.local/share/nvim/site/pack/packer/start/gitsigns.nvim/ftdetect
allow ${HOME}/.local/share/nvim/site/pack/packer/start/coq_nvim/ftdetect
allow ${HOME}/.local/share/nvim/site/pack/packer/start/coq_nvim/ftdetect/
allow ${HOME}/.local/share/nvim/site/pack/packer/start/coq.artifacts/ftdetect
allow ${HOME}/.local/share/nvim/site/ftdetect
allow ${HOME}/.local/share/nvim/site/pack/packer/start/material.nvim/lua/material
allow ${HOME}/.local/share/nvim/site/pack/packer/start/material.nvim/colors
allow ${HOME}/.local/share/nvim/site/pack/packer/start/telescope-project.nvim/lua/telescope/_extensions
allow ${HOME}/.local/share/nvim/site/pack/packer/start/telescope-fzf-native.nvim/lua
allow ${HOME}/.local/share/nvim/site/pack/packer/start/telescope-fzf-native.nvim/lua/telescope/_extensions
allow ${HOME}/.local/share/nvim/site/pack/packer/start/plenary.nvim/lua/plenary
allow ${HOME}/.local/share/nvim/site/pack/packer/start/telescope.nvim/lua/telescope
allow ${HOME}/.local/share/nvim/site/pack/packer/start/nvim-lspconfig/lua
allow ${HOME}/.local/share/nvim/site/pack/packer/start/packer.nvim/lua
allow ${HOME}/.local/share/nvim/site/after,/etc/xdg/nvim/after,/home/misterspoon/.config/kdedefaults/nvim/after,/home/misterspoon/.config/nvim/after/start
allow ${HOME}/.local/share/nvim/site/after,/etc/xdg/nvim/after,/home/misterspoon/.config/kdedefaults/nvim/after,/home/misterspoon/.config/nvim/after/pack
allow ${HOME}/.local/nvim/lib/nvim/start
allow ${HOME}/.local/nvim/lib/nvim/pack
allow ${HOME}/.local/share/nvim/site/start
allow ${HOME}/.local/share/nvim/site/pack/packer/start/
allow ${HOME}/.local/share/nvim/site/pack/
allow ${HOME}/.config/nvim
allow ${HOME}/.local/nvim/share/nvim
allow ${HOME}/.config/kdedefaults/nvim
allow ${HOME}/.terminfo/78
allow ${HOME}/.terminfo/x
allow ${HOME}/.cache/nvim
include whitelist-common.inc

### Filesystem Whitelisting ###
allow /usr/share/nvim
allow /usr/share/terminfo
include whitelist-usr-share-common.inc
include whitelist-var-common.inc

#apparmor	# if you have AppArmor running, try this one!
caps.drop all
ipc-namespace
netfilter
#no3d	# disable 3D acceleration
#nodvd	# disable DVD and CD devices
#nogroups	# disable supplementary user groups
#noinput	# disable input devices
nonewprivs
noroot
#notv	# disable DVB TV devices
#nou2f	# disable U2F devices
#novideo	# disable video capture devices
protocol unix,
net none
seccomp
shell none
tracelog

#disable-mnt	# no access to /mnt, /media, /run/mount and /run/media
private-bin tmux,python3.9,git
#private-cache	# run with an empty ~/.cache directory
private-dev
private-etc gitconfig,xdg,
#private-lib
#private-tmp
# File accessed in /tmp directory:
# /tmp/nvimKbSKzZ/,

#dbus-user none
#dbus-system none

#memory-deny-write-execute

kmk3 added a commit to kmk3/firejail that referenced this issue Oct 5, 2021
@kmk3
Revert "allow/deny fbuilder"
3c89c50 
This reverts commit 4438f14.

Also, partially revert related commit e4307b4 ("fix whitelist/allow in
make test-utils") to keep tests working.

The profiles are being generated using aliases, which are not documented
nor used on the profiles in the repository.  So generate them using the
normal commands for consistency.  See also commit dd13595 ("Revert
"allow/deny help and man pages"") / PR netblue30#4502.

Relates to netblue30#4410.

Misc: I noticed this on issue netblue30#4592.
@kmk3 kmk3 mentioned this issue Oct 5, 2021
Merged
@matu3ba
Copy link
Contributor Author

matu3ba commented Oct 6, 2021 •
edited
Loading

looks like a wild pointer to me. The only occurence of after is in build_profile.c on line 95: printf("--- Built profile beings after this line ---\n");
Is there a way to run firejail within valgrind or do I need to use gdb for this?

UPDATED description.

@rusty-snake
Copy link
Collaborator

Is there a way to run firejail within valgrind or do I need to use gdb for this?

Maybe this is helpful https://github.com/netblue30/firejail/wiki/Debugging-Firejail#advanced-troubleshooting.

ASAN build would be nice too however I did not make it work so far.

@smitsohu
Copy link
Collaborator

smitsohu commented Oct 6, 2021

Just run fbuilder manually. It will be much easier to debug.

/usr/lib64/firejail/fbuilder nvim (probably)

@smitsohu
Copy link
Collaborator

smitsohu commented Oct 6, 2021

ASAN build would be nice too however I did not make it work so far.

+1
I also tried a while back but didn't make it past an endless loop in the instrumented binary.

@matu3ba
Copy link
Contributor Author

matu3ba commented Oct 6, 2021 •
edited
Loading

nvm. I should not use mold for this stuff.

The binary seems very messed up. valgrind cant read the debug sections. Can you confirm this?
Changing common.mk by removing -O2 did not change this for me.

objdump -h fbuilder returns very oddly looking 2 .rodata sections

12 .rodata       00000202  0000000000001a50  0000000000001a50  00001a50  2**4
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
13 .rodata       00001260  0000000000001c60  0000000000001c60  00001c60  2**4
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
fbuilder:     file format elf64-x86-64

Sections:
Idx Name          Size      VMA               LMA               File off  Algn
  0 .interp       0000001c  0000000000000270  0000000000000270  00000270  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  1 .note.gnu.build-id 00000024  000000000000028c  000000000000028c  0000028c  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  2 .note.ABI-tag 00000020  00000000000002b0  00000000000002b0  000002b0  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  3 .dynsym       000003c0  0000000000000618  0000000000000618  00000618  2**3
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  4 .rela.plt     00000348  00000000000002d0  00000000000002d0  000002d0  2**3
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  5 .dynstr       000001ce  00000000000009d8  00000000000009d8  000009d8  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  6 .eh_frame     00000838  0000000000000ba8  0000000000000ba8  00000ba8  2**3
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  7 .eh_frame_hdr 00000114  00000000000013e0  00000000000013e0  000013e0  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  8 .gnu.hash     00000100  00000000000014f8  00000000000014f8  000014f8  2**3
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  9 .rela.dyn     00000360  00000000000015f8  00000000000015f8  000015f8  2**3
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
 10 .gnu.version  00000050  0000000000001958  0000000000001958  00001958  2**1
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
 11 .gnu.version_r 000000a0  00000000000019a8  00000000000019a8  000019a8  2**3
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
 12 .rodata       00000202  0000000000001a50  0000000000001a50  00001a50  2**4
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
 13 .rodata       00001260  0000000000001c60  0000000000001c60  00001c60  2**4
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
 14 .plt          00000240  0000000000003000  0000000000003000  00003000  2**4
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
 15 .fini         0000000d  0000000000003240  0000000000003240  00003240  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
 16 .init         0000001b  0000000000003250  0000000000003250  00003250  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
 17 .text         00003205  0000000000003270  0000000000003270  00003270  2**4
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
 18 .got          00000028  0000000000007000  0000000000007000  00007000  2**3
                  CONTENTS, ALLOC, LOAD, DATA
 19 .dynamic      00000220  0000000000007028  0000000000007028  00007028  2**3
                  CONTENTS, ALLOC, LOAD, DATA
 20 .data.rel.ro  000000f0  0000000000007260  0000000000007260  00007260  2**5
                  CONTENTS, ALLOC, LOAD, DATA
 21 .fini_array   00000008  0000000000007350  0000000000007350  00007350  2**3
                  CONTENTS, ALLOC, LOAD, DATA
 22 .init_array   00000008  0000000000007358  0000000000007358  00007358  2**3
                  CONTENTS, ALLOC, LOAD, DATA
 23 .got.plt      00000130  0000000000008000  0000000000008000  00008000  2**3
                  CONTENTS, ALLOC, LOAD, DATA
 24 .data         00000010  0000000000008130  0000000000008130  00008130  2**3
                  CONTENTS, ALLOC, LOAD, DATA
 25 .tm_clone_table 00000000  0000000000008140  0000000000008140  00008140  2**3
                  CONTENTS, ALLOC, LOAD, DATA
 26 .dynbss       00000048  0000000000008140  0000000000008140  00008140  2**6
                  ALLOC
 27 .bss          00000074  0000000000008188  0000000000008188  00008140  2**3
                  ALLOC
 28 .comment      0000006e  0000000000000000  0000000000000000  00008140  2**0
                  CONTENTS, READONLY

and valgrind has problem to read the debug data:
valgrind --leak-check=full -s ./fbuilder ~/.local/nvim/bin/nvim +q

==277551== Memcheck, a memory error detector
==277551== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==277551== Using Valgrind-3.17.0 and LibVEX; rerun with -h for copyright info
==277551== Command: ./fbuilder /home/misterspoon/.local/nvim/bin/nvim
==277551==
--277551-- WARNING: Serious error when reading debug info
--277551-- When reading debug info from /usr/local/lib/firejail/fbuilder:
--277551-- Can't make sense of .rodata section mapping
--- Built profile beings after this line ---
# Save this file as "application.profile" (change "application" with the
# program name) in ~/.config/firejail directory. Firejail will find it
# automatically every time you sandbox your application.
#
# Run "firejail application" to test it. In the file there are
# some other commands you can try. Enable them by removing the "#".

# Firejail profile for /home/misterspoon/.local/nvim/bin/nvim
# Persistent local customizations
#include /home/misterspoon/.local/nvim/bin/nvim.local
# Persistent global definitions
#include globals.local

### Basic Blacklisting ###
### Enable as many of them as you can! A very important one is
### "disable-exec.inc". This will make among other things your home
### and /tmp directories non-executable.
include disable-common.inc	# dangerous directories like ~/.ssh and ~/.gnupg
#include disable-devel.inc	# development tools such as gcc and gdb
#include disable-exec.inc	# non-executable directories such as /var, /tmp, and /home
#include disable-interpreters.inc	# perl, python, lua etc.
include disable-programs.inc	# user configuration for programs such as firefox, vlc etc.
#include disable-shell.inc	# sh, bash, zsh etc.
#include disable-xdg.inc	# standard user directories: Documents, Pictures, Videos, Music

### Home Directory Whitelisting ###
### If something goes wrong, this section is the first one to comment out.
### Instead, you'll have to relay on the basic blacklisting above.
allow ${HOME}/.local/nvim/lib/nvim
allow ${HOME}/.gitconfig
allow ${HOME}/.config/git
allow ${HOME}/.local/nvim/lib/nvim/plugin
allow ${HOME}/.local/share/nvim
allow ${HOME}/.local/share/nvim/site/plugin
allow ${HOME}/.local/share/nvim/site/after/pack
allow ${HOME}/.local/share/nvim/site/after/start
allow ${HOME}/.local/nvim/lib/nvim/ftdetect
allow ${HOME}/.local/share/nvim/site/ftdetect
allow ${HOME}/.local/share/nvim/site/after,/etc/xdg/nvim/after,/home/misterspoon/.config/kdedefaults/nvim/after,/home/misterspoon/.config/nvim/after/start
allow ${HOME}/.local/share/nvim/site/after,/etc/xdg/nvim/after,/home/misterspoon/.config/kdedefaults/nvim/after,/home/misterspoon/.config/nvim/after/pack
allow ${HOME}/.local/nvim/lib/nvim/start
allow ${HOME}/.local/nvim/lib/nvim/pack
allow ${HOME}/.local/share/nvim/site/start
allow ${HOME}/.local/share/nvim/site/pack
allow ${HOME}/.config/nvim
allow ${HOME}/.local/nvim/share/nvim
allow ${HOME}/.config/kdedefaults/nvim
allow ${HOME}/.terminfo/78
allow ${HOME}/.terminfo/x
allow ${HOME}/.cache/nvim
include whitelist-common.inc

### Filesystem Whitelisting ###
allow /usr/share/nvim
allow /usr/share/terminfo
include whitelist-usr-share-common.inc
include whitelist-var-common.inc

#apparmor	# if you have AppArmor running, try this one!
caps.drop all
ipc-namespace
netfilter
#no3d	# disable 3D acceleration
#nodvd	# disable DVD and CD devices
#nogroups	# disable supplementary user groups
#noinput	# disable input devices
nonewprivs
noroot
#notv	# disable DVB TV devices
#nou2f	# disable U2F devices
#novideo	# disable video capture devices
protocol unix,
net none
seccomp
shell none
tracelog

#disable-mnt	# no access to /mnt, /media, /run/mount and /run/media
private-bin tmux,python3.9,git,
#private-cache	# run with an empty ~/.cache directory
private-dev
private-etc gitconfig,xdg,
#private-lib
#private-tmp
# File accessed in /tmp directory:
# /tmp/nvim9GQeu7/,

#dbus-user none
#dbus-system none

#memory-deny-write-execute
==277551==
==277551== HEAP SUMMARY:
==277551==     in use at exit: 1,559 bytes in 61 blocks
==277551==   total heap usage: 2,526 allocs, 2,465 frees, 307,669 bytes allocated
==277551==
==277551== 35 bytes in 1 blocks are definitely lost in loss record 19 of 21
==277551==    at 0x483E7C5: malloc (vg_replace_malloc.c:380)
==277551==    by 0x4911C5F: __vasprintf_internal (in /usr/lib/libc-2.33.so)
==277551==    by 0x49A18C2: __asprintf_chk (in /usr/lib/libc-2.33.so)
==277551==    by 0x10CA1E: ??? (in /usr/local/lib/firejail/fbuilder)
==277551==    by 0x10E07C: ??? (in /usr/local/lib/firejail/fbuilder)
==277551==    by 0x1FFF000575: ???
==277551==    by 0x656D6F682F007264: ???
==277551==    by 0x7372657473696D2E: ???
==277551==    by 0x6F6C2E2F6E6F6F6F: ???
==277551==    by 0x6D69766E2F6C6162: ???
==277551==    by 0x69766E2F6E69622E: ???
==277551==    by 0x3D4C4C454853006C: ???
==277551==
==277551== LEAK SUMMARY:
==277551==    definitely lost: 35 bytes in 1 blocks
==277551==    indirectly lost: 0 bytes in 0 blocks
==277551==      possibly lost: 0 bytes in 0 blocks
==277551==    still reachable: 1,524 bytes in 60 blocks
==277551==         suppressed: 0 bytes in 0 blocks
==277551== Reachable blocks (those to which a pointer was found) are not shown.
==277551== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==277551==
==277551== For lists of detected and suppressed errors, rerun with: -s
==277551== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
[misterspoon@pc firejail]$ valgrind --leak-check=full -s ./fbuilder ~/.local/nvim/bin/nvim +q
==277595== Memcheck, a memory error detector
==277595== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==277595== Using Valgrind-3.17.0 and LibVEX; rerun with -h for copyright info
==277595== Command: ./fbuilder /home/misterspoon/.local/nvim/bin/nvim +q
==277595==
--277595-- WARNING: Serious error when reading debug info
--277595-- When reading debug info from /usr/local/lib/firejail/fbuilder:
--277595-- Can't make sense of .rodata section mapping
Error: cannot run the sandbox
==277595==
==277595== HEAP SUMMARY:
==277595==     in use at exit: 35 bytes in 1 blocks
==277595==   total heap usage: 2 allocs, 1 frees, 135 bytes allocated
==277595==
==277595== LEAK SUMMARY:
==277595==    definitely lost: 0 bytes in 0 blocks
==277595==    indirectly lost: 0 bytes in 0 blocks
==277595==      possibly lost: 0 bytes in 0 blocks
==277595==    still reachable: 35 bytes in 1 blocks
==277595==         suppressed: 0 bytes in 0 blocks
==277595== Reachable blocks (those to which a pointer was found) are not shown.
==277595== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==277595==
==277595== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

@matu3ba matu3ba mentioned this issue Oct 6, 2021
Closed
kmk3 added a commit to kmk3/firejail that referenced this issue Oct 6, 2021
@kmk3
Revert "allow/deny fbuilder"
a7e4b9b 
This reverts commit 4438f14.

Also, partially revert related commit e4307b4 ("fix whitelist/allow in
make test-utils") to keep the tests working.

The profiles are being generated using aliases, which are not used on
the profiles in the repository.  So generate them using the normal
commands for consistency.  See also commit dd13595 ("Revert
"allow/deny help and man pages"") / PR netblue30#4502.

Relates to netblue30#4410.

Misc: I noticed this on issue netblue30#4592.
@reinerh reinerh mentioned this issue Oct 6, 2021
Merged
@rusty-snake
Copy link
Collaborator

Building with -fanalyzer (gcc (GCC) 11.2.1 20210728 (Red Hat 11.2.1-1)) finds

filedb.c:71:22: error: use of possibly-NULL ‘strdup(fname)’ where non-null expected [CWE-690] [-Werror=analyzer-possible-null-argument] 
filedb.c: In function ‘filedb_add.part.0’:
filedb.c:71:22: error: use of possibly-NULL ‘strdup(fname)’ where non-null expected [CWE-690] [-Werror=analyzer-possible-null-argument]
   71 |         entry->len = strlen(entry->fname);
      |                      ^~~~~~~~~~~~~~~~~~~~
  ‘filedb_load_whitelist’: event 1
    |
    |   90 | FileDB *filedb_load_whitelist(FileDB *head, const char *fname, const char *prefix) {
    |      |         ^~~~~~~~~~~~~~~~~~~~~
    |      |         |
    |      |         (1) entry to ‘filedb_load_whitelist’
    |
  ‘filedb_load_whitelist’: event 2
    |
    |   91 |         assert(fname);
    |      |         ^~~~~~
    |      |         |
    |      |         (2) following ‘true’ branch (when ‘fname’ is non-NULL)...
    |
  ‘filedb_load_whitelist’: event 3
    |
    |   92 |         assert(prefix);
    |      |         ^~~~~~
    |      |         |
    |      |         (3) ...to here
    |
  ‘filedb_load_whitelist’: event 4
    |
    |   92 |         assert(prefix);
    |      |         ^~~~~~
    |      |         |
    |      |         (4) following ‘true’ branch (when ‘prefix’ is non-NULL)...
    |
  ‘filedb_load_whitelist’: events 5-6
    |
    |   93 |         int len = strlen(prefix);
    |      |         ^~~
    |      |         |
    |      |         (5) ...to here
    |   94 |         char *f;
    |   95 |         if (asprintf(&f, "%s/%s", SYSCONFDIR, fname) == -1)
    |      |            ~
    |      |            |
    |      |            (6) following ‘false’ branch...
    |
  ‘filedb_load_whitelist’: event 7
    |
    |../include/common.h:39:28:
    |   39 | #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
filedb.c:96:17: note: in expansion of macro ‘errExit’
    |   96 |                 errExit("asprintf");
    |      |                 ^~~~~~~
    |
  ‘filedb_load_whitelist’: event 8
    |
    |   98 |         if (!fp) {
    |      |            ^
    |      |            |
    |      |            (8) following ‘false’ branch (when ‘fp’ is non-NULL)...
    |
  ‘filedb_load_whitelist’: event 9
    |
    |cc1:
    | (9): ...to here
    |
  ‘filedb_load_whitelist’: events 10-16
    |
    |  105 |         while (fgets(buf, MAX_BUF, fp)) {
    |      |                ^~~~~
    |      |                |
    |      |                (10) following ‘true’ branch...
    |  106 |                 if (strncmp(buf, prefix, len) != 0)
    |      |                 ~~ ~
    |      |                 |  |
    |      |                 |  (12) following ‘false’ branch...
    |      |                 (11) ...to here
    |......
    |  109 |                 char *fn = buf + len;
    |      |                 ~~~~
    |      |                 |
    |      |                 (13) ...to here
    |  110 |                 char *ptr = strchr(buf, '\n');
    |  111 |                 if (!ptr)
    |      |                    ~
    |      |                    |
    |      |                    (14) following ‘false’ branch (when ‘ptr’ is non-NULL)...
    |  112 |                         continue;
    |  113 |                 *ptr = '\0';
    |      |                 ~
    |      |                 |
    |      |                 (15) ...to here
    |......
    |  116 |                 head = filedb_add(head, fn);
    |      |                        ~~~~~~~~~~~~~~~~~~~~
    |      |                        |
    |      |                        (16) calling ‘filedb_add’ from ‘filedb_load_whitelist’
    |
    +--> ‘filedb_add’: event 17
           |
           |   54 | FileDB *filedb_add(FileDB *head, const char *fname) {
           |      |         ^~~~~~~~~~
           |      |         |
           |      |         (17) entry to ‘filedb_add’
           |
         ‘filedb_add’: event 18
           |
           |   55 |         assert(fname);
           |      |         ^~~~~~
           |      |         |
           |      |         (18) following ‘true’ branch (when ‘fname’ is non-NULL)...
           |
         ‘filedb_add’: events 19-20
           |
           |   60 |         if (filedb_find(head, fname))
           |      |         ^~  ~~~~~~~~~~~~~~~~~~~~~~~~
           |      |         |   |
           |      |         |   (20) calling ‘filedb_find’ from ‘filedb_add’
           |      |         (19) ...to here
           |
           +--> ‘filedb_find’: event 21
                  |
                  |   24 | FileDB *filedb_find(FileDB *head, const char *fname) {
                  |      |         ^~~~~~~~~~~
                  |      |         |
                  |      |         (21) entry to ‘filedb_find’
                  |
                ‘filedb_find’: event 22
                  |
                  |   25 |         assert(fname);
                  |      |         ^~~~~~
                  |      |         |
                  |      |         (22) following ‘true’ branch (when ‘fname’ is non-NULL)...
                  |
                ‘filedb_find’: events 23-29
                  |
                  |   26 |         FileDB *ptr = head;
                  |      |         ^~~~~~
                  |      |         |
                  |      |         (23) ...to here
                  |......
                  |   30 |         while (ptr) {
                  |      |                ~~~
                  |      |                |
                  |      |                (24) following ‘true’ branch (when ‘ptr’ is non-NULL)...
                  |      |                (26) following ‘true’ branch (when ‘ptr’ is non-NULL)...
                  |   31 |                 // exact name
                  |   32 |                 if (strcmp(fname, ptr->fname) == 0) {
                  |      |                 ~~
                  |      |                 |
                  |      |                 (25) ...to here
                  |      |                 (27) ...to here
                  |......
                  |   48 |         if (found)
                  |      |            ~
                  |      |            |
                  |      |            (28) following ‘true’ branch (when ‘found != 0’)...
                  |   49 |                 return ptr;
                  |      |                 ~~~~~~
                  |      |                 |
                  |      |                 (29) ...to here
                  |
           <------+
           |
         ‘filedb_add’: events 30-31
           |
           |   60 |         if (filedb_find(head, fname))
           |      |            ~^~~~~~~~~~~~~~~~~~~~~~~~
           |      |            ||
           |      |            |(30) returning to ‘filedb_add’ from ‘filedb_find’
           |      |            (31) following ‘false’ branch...
           |
         ‘filedb_add’: event 32
           |
           |cc1:
           | (32): ...to here
           |
         ‘filedb_add’: event 33
           |
           |cc1:
           | (33): calling ‘filedb_add.part.0’ from ‘filedb_add’
           |
           +--> ‘filedb_add.part.0’: events 34-35
                  |
                  |   54 | FileDB *filedb_add(FileDB *head, const char *fname) {
                  |      |         ^~~~~~~~~~
                  |      |         |
                  |      |         (34) entry to ‘filedb_add.part.0’
                  |......
                  |   65 |         if (!entry)
                  |      |            ~
                  |      |            |
                  |      |            (35) following ‘false’ branch (when ‘entry’ is non-NULL)...
                  |
                ‘filedb_add.part.0’: event 36
                  |
                  |../include/common.h:39:28:
                  |   39 | #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)
                  |      |                            ^
                  |      |                            |
                  |      |                            (36) ...to here
filedb.c:66:17: note: in expansion of macro ‘errExit’
                  |   66 |                 errExit("malloc");
                  |      |                 ^~~~~~~
                  |
                ‘filedb_add.part.0’: events 37-39
                  |
                  |   68 |         entry->fname = strdup(fname);
                  |      |                        ^~~~~~~~~~~~~
                  |      |                        |
                  |      |                        (37) this call could return NULL
                  |   69 |         if (!entry->fname)
                  |      |            ~            
                  |      |            |
                  |      |            (38) assuming ‘strdup(fname)’ is non-NULL
                  |      |            (39) following ‘false’ branch...
                  |
                ‘filedb_add.part.0’: event 40
                  |
                  |../include/common.h:39:28:
                  |   39 | #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)
                  |      |                            ^
                  |      |                            |
                  |      |                            (40) ...to here
filedb.c:70:17: note: in expansion of macro ‘errExit’
                  |   70 |                 errExit("strdup");
                  |      |                 ^~~~~~~
                  |
           <------+
           |
         ‘filedb_add’: event 41
           |
           |cc1:
           | (41): returning to ‘filedb_add’ from ‘filedb_add.part.0’
           |
    <------+
    |
  ‘filedb_load_whitelist’: events 42-49
    |
    |  105 |         while (fgets(buf, MAX_BUF, fp)) {
    |      |                ~~~~~    
    |      |                |
    |      |                (43) following ‘true’ branch...
    |  106 |                 if (strncmp(buf, prefix, len) != 0)
    |      |                 ~~ ~    
    |      |                 |  |
    |      |                 |  (45) following ‘false’ branch...
    |      |                 (44) ...to here
    |......
    |  109 |                 char *fn = buf + len;
    |      |                 ~~~~    
    |      |                 |
    |      |                 (46) ...to here
    |  110 |                 char *ptr = strchr(buf, '\n');
    |  111 |                 if (!ptr)
    |      |                    ~    
    |      |                    |
    |      |                    (47) following ‘false’ branch (when ‘ptr’ is non-NULL)...
    |  112 |                         continue;
    |  113 |                 *ptr = '\0';
    |      |                 ~       
    |      |                 |
    |      |                 (48) ...to here
    |......
    |  116 |                 head = filedb_add(head, fn);
    |      |                        ^~~~~~~~~~~~~~~~~~~~
    |      |                        |
    |      |                        (42) returning to ‘filedb_load_whitelist’ from ‘filedb_add’
    |      |                        (49) calling ‘filedb_add’ from ‘filedb_load_whitelist’
    |
    +--> ‘filedb_add’: event 50
           |
           |   54 | FileDB *filedb_add(FileDB *head, const char *fname) {
           |      |         ^~~~~~~~~~
           |      |         |
           |      |         (50) entry to ‘filedb_add’
           |
         ‘filedb_add’: event 51
           |
           |   55 |         assert(fname);
           |      |         ^~~~~~
           |      |         |
           |      |         (51) following ‘true’ branch (when ‘fname’ is non-NULL)...
           |
         ‘filedb_add’: events 52-53
           |
           |   60 |         if (filedb_find(head, fname))
           |      |         ^~  ~~~~~~~~~~~~~~~~~~~~~~~~
           |      |         |   |
           |      |         |   (53) calling ‘filedb_find’ from ‘filedb_add’
           |      |         (52) ...to here
           |
           +--> ‘filedb_find’: event 54
                  |
                  |   24 | FileDB *filedb_find(FileDB *head, const char *fname) {
                  |      |         ^~~~~~~~~~~
                  |      |         |
                  |      |         (54) entry to ‘filedb_find’
                  |
                ‘filedb_find’: event 55
                  |
                  |   25 |         assert(fname);
                  |      |         ^~~~~~
                  |      |         |
                  |      |         (55) following ‘true’ branch (when ‘fname’ is non-NULL)...
                  |
                ‘filedb_find’: events 56-61
                  |
                  |   26 |         FileDB *ptr = head;
                  |      |         ^~~~~~
                  |      |         |
                  |      |         (56) ...to here
                  |......
                  |   30 |         while (ptr) {
                  |      |                ~~~
                  |      |                |
                  |      |                (57) following ‘true’ branch (when ‘ptr’ is non-NULL)...
                  |   31 |                 // exact name
                  |   32 |                 if (strcmp(fname, ptr->fname) == 0) {
                  |      |                 ~~ ~
                  |      |                 |  |
                  |      |                 |  (59) following ‘false’ branch (when the strings are non-equal)...
                  |      |                 (58) ...to here
                  |......
                  |   38 |                 if (len > ptr->len &&
                  |      |                 ~~
                  |      |                 |
                  |      |                 (60) ...to here
                  |......
                  |   48 |         if (found)
                  |      |            ~
                  |      |            |
                  |      |            (61) following ‘false’ branch (when ‘found == 0’)...
                  |
                ‘filedb_find’: event 62
                  |
                  |cc1:
                  | (62): ...to here
                  |
           <------+
           |
         ‘filedb_add’: events 63-64
           |
           |   60 |         if (filedb_find(head, fname))
           |      |            ~^~~~~~~~~~~~~~~~~~~~~~~~
           |      |            ||
           |      |            |(63) returning to ‘filedb_add’ from ‘filedb_find’
           |      |            (64) following ‘false’ branch...
           |
         ‘filedb_add’: event 65
           |
           |cc1:
           | (65): ...to here
           |
         ‘filedb_add’: event 66
           |
           |cc1:
           | (66): calling ‘filedb_add.part.0’ from ‘filedb_add’
           |
           +--> ‘filedb_add.part.0’: events 67-68
                  |
                  |   54 | FileDB *filedb_add(FileDB *head, const char *fname) {
                  |      |         ^~~~~~~~~~
                  |      |         |
                  |      |         (67) entry to ‘filedb_add.part.0’
                  |......
                  |   65 |         if (!entry)
                  |      |            ~
                  |      |            |
                  |      |            (68) following ‘false’ branch (when ‘entry’ is non-NULL)...
                  |
                ‘filedb_add.part.0’: event 69
                  |
                  |../include/common.h:39:28:
                  |   39 | #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)
                  |      |                            ^
                  |      |                            |
                  |      |                            (69) ...to here
filedb.c:66:17: note: in expansion of macro ‘errExit’
                  |   66 |                 errExit("malloc");
                  |      |                 ^~~~~~~
                  |
                ‘filedb_add.part.0’: events 70-71
                  |
                  |   68 |         entry->fname = strdup(fname);
                  |      |                        ^~~~~~~~~~~~~
                  |      |                        |
                  |      |                        (70) this call could return NULL
                  |   69 |         if (!entry->fname)
                  |      |            ~            
                  |      |            |
                  |      |            (71) following ‘false’ branch...
                  |
                ‘filedb_add.part.0’: event 72
                  |
                  |../include/common.h:39:28:
                  |   39 | #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)
                  |      |                            ^
                  |      |                            |
                  |      |                            (72) ...to here
filedb.c:70:17: note: in expansion of macro ‘errExit’
                  |   70 |                 errExit("strdup");
                  |      |                 ^~~~~~~
                  |
                ‘filedb_add.part.0’: event 73
                  |
                  |   71 |         entry->len = strlen(entry->fname);
                  |      |                      ^~~~~~~~~~~~~~~~~~~~
                  |      |                      |
                  |      |                      (73) argument 1 (‘strdup(fname)’) from (70) could be NULL where non-null expected
                  |
In file included from ../include/common.h:31,
                 from fbuilder.h:23,
                 from filedb.c:21:
/usr/include/string.h:391:15: note: argument 1 of ‘strlen’ must be non-null
  391 | extern size_t strlen (const char *__s)
      |               ^~~~~~

@rusty-snake rusty-snake mentioned this issue Oct 29, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@matu3ba @smitsohu @rusty-snake