free(): invalid pointer with --ip=dhcp

[kris7t]

Bug and expected behavior
When running Firejail with --ip=dhcp, it dumps core after the jailed application exits normally (here /usr/bin/true is used as an program that exits immediately):

$ firejail --net=br-untrusted --ip=dhcp --noprofile /usr/bin/true
Parent pid 2198151, child pid 2198153

Interface        MAC                IP               Mask             Status
lo                                  127.0.0.1        255.0.0.0        UP

Child process initialized in 627.41 ms
free(): invalid pointer
[1]    2198151 abort (core dumped)  firejail --net=br-untrusted --ip=dhcp --noprofile /usr/bin/true

The core dump is caused by line 3088 of firejail/main.c:

$ coredumpctl gdb
           PID: 2198151 (firejail)
           UID: 1000 (kris)
           GID: 1000 (kris)
        Signal: 6 (ABRT)
     Timestamp: Sat 2020-10-10 21:06:07 CEST (2s ago)
  Command Line: firejail --net=br-untrusted --ip=dhcp --noprofile /usr/bin/true
    Executable: /usr/bin/firejail
 Control Group: /user.slice/user-1000.slice/session-3.scope
          Unit: session-3.scope
         Slice: user-1000.slice
       Session: 3
     Owner UID: 1000 (kris)
       Boot ID: <REDACTED>
    Machine ID: <REDACTED>
      Hostname: blushweaver.marussy.com
       Storage: /var/lib/systemd/coredump/core.firejail.1000.28030ec480a04a598419e379fbeb7cc7.2198151.1602356767000000.zst
       Message: Process 2198151 (firejail) of user 1000 dumped core.

                Stack trace of thread 2198151:
                #0  0x00007fc5f9472615 raise (libc.so.6 + 0x3d615)
                #1  0x00007fc5f945b862 abort (libc.so.6 + 0x26862)
                #2  0x00007fc5f94b45e8 __libc_message (libc.so.6 + 0x7f5e8)
                #3  0x00007fc5f94bc27a malloc_printerr (libc.so.6 + 0x8727a)
                #4  0x00007fc5f94bd64c _int_free (libc.so.6 + 0x8864c)
                #5  0x0000564cce1be0fe main (firejail + 0xd0fe)
                #6  0x00007fc5f945d152 __libc_start_main (libc.so.6 + 0x28152)
                #7  0x0000564cce1c487e _start (firejail + 0x1387e)

GNU gdb (GDB) 9.2
Copyright (C) 2020 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /usr/bin/firejail...
[New LWP 2198151]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/libthread_db.so.1".
Core was generated by `firejail --net=br-untrusted --ip=dhcp --noprofile /usr/bin/true'.
Program terminated with signal SIGABRT, Aborted.
#0  0x00007fc5f9472615 in raise () from /usr/lib/libc.so.6
(gdb) ba
#0  0x00007fc5f9472615 in raise () from /usr/lib/libc.so.6
#1  0x00007fc5f945b862 in abort () from /usr/lib/libc.so.6
#2  0x00007fc5f94b45e8 in __libc_message () from /usr/lib/libc.so.6
#3  0x00007fc5f94bc27a in malloc_printerr () from /usr/lib/libc.so.6
#4  0x00007fc5f94bd64c in _int_free () from /usr/lib/libc.so.6
#5  0x0000564cce1be0fe in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at main.c:3088

Seems like we're trying to free a profile line that was already freed:

firejail/src/firejail/main.c

Lines 3083 to 3093 in 7fb7e42

	// free globals
	if (cfg.profile) {
	ProfileEntry *prf = cfg.profile;
	while (prf != NULL) {
	ProfileEntry *next = prf->next;
	free(prf->data);
	free(prf->link);
	free(prf);
	prf = next;
	}
	}

Oddly enough, I don't have the same crash with --ip6=dhcp (i.e. use ARP scanning to determine the IPv4 address, but use dhcpcd for IPv6).

According to git bisect, the commit that introduced the crash is 81b2c7a.

Reproduce
Steps to reproduce the behavior:

1. Run in bash firejail with --ip=dhcp.
2. Let the jailed program exit.
3. See the core being dumped.

Environment

• Arch Linux Linux 5.8.13-arch1-1 #1 SMP PREEMPT Thu, 01 Oct 2020 20:40:35 +0000 x86_64 GNU/Linux
• On firejail git commit 9bf6e0ead189b924e5fca099b35d88be091bd009

[kris7t]

@netblue30 I have a bit of a hard time understanding this:

firejail/src/firejail/main.c

Lines 1050 to 1057 in f373fe1

	// --ip=dhcp - we need access to /sbin and /usr/sbin directories in order to run ISC DHCP client (dhclient)
	// these paths are disabled in disable-common.inc
	if ((i = check_arg(argc, argv, "--ip", 0)) != 0) {
	if (strncmp(argv[i] + 4, "=dhcp", 5) == 0) {
	profile_add("noblacklist /sbin");
	profile_add("noblacklist /usr/sbin");
	}
	}

1. Doesn't profile_add take ownership of its argument? Maybe we'd need something like profile_add_dup, which first calls strdup on its argument and then passes it to profile_add (lest we try to free a string literal later when the profile entries are freed).
2. Do we need to noblacklist /sbin before parsing the rest of the arguments? I am on Arch where /sbin and /usr/sbin are just symlinks to /usr/bin, so I can't really test this.
3. Would we also want to do the same thing for --ip6=dhcp? What about ip dhcp and ip6 dhcp in profiles?

[rusty-snake]

2. blacklist follows symliks. Test with firejail --noprofile --blacklist=/sbin ls -ld /usr/sbin.

[netblue30]

The profile free code is 6 years old, the original intent was long lost. We put it in to keep tools like valgrind quiet, but on the way valgind got broken. I commented it out for now, it doesn't make sense in this moment since the sandbox is already closed.