configuration file without pid causes "Bad magic in options.c, line 1059" crash on SIGHUP

[Fredrick Knieper]

In focal/universe (Ubuntu 20.04.1 LTS), stunnel4 version 3:5.56-1 has a build patch named

04-restore-pidfile-default.patch

This patch defines a constant string PIDFILE and uses this constant string in the lines:

    case CMD_SET_DEFAULTS:
        new_global_options.pidfile=PIDFILE;

(options.c line 1051-1052) after patch.

When SIGHUP is received, stunnel reloads this configuration file. During this process, it attempts to free the pidfile path string from the current configuration:

    case CMD_FREE:
        tmp=global_options.pidfile;
        global_options.pidfile=NULL;
        str_free(tmp);

Here, attempting to str_free() the string literal triggers the "Bad magic" error and crashes stunnel. I *believe* the correct fix is to modify patch #04 to read:

    case CMD_SET_DEFAULTS:
        new_global_options.pidfile=str_dup(PIDFILE);

based on the handling of other arguments elsewhere in this function, but I am not sufficiently versed in C programming to know for sure that this is correct and that it will not introduce other errors (for instance, if the default PIDFILE literal will be re-loaded).

Additionally, the stunnel4 manpage does not indicate that `pid` is a required setting, only that leaving it blank prevents a pidfile from being created. This bug was encountered using Amazon Elastic File System's `mount.efs` helper that autogenerates a stunnel configuration, launches stunnel with this configuration, then SIGHUP's that stunnel every hour for certificate changes.

Reproduction:

min.config
==========
foreground=yes
client=yes
[test]
accept = 127.0.0.1:5001
connect =127.0.0.1:5002

1. Execute `stunnel4 min.config`
2. send a SIGHUP to that stunnel process

Expected behavior: configuration reloaded and stunnel execution continues

Observed behavior:
2020.10.27 23:16:23 LOG5[ui]: Reading configuration from file /root/min.config
2020.10.27 23:16:23 LOG5[ui]: UTF-8 byte order mark not detected
2020.10.27 23:16:23 LOG5[ui]: FIPS mode disabled
2020.10.27 23:16:23 LOG4[ui]: Service [test] needs authentication to prevent MITM attacks
2020.10.27 23:16:23 LOG5[ui]: Configuration successful
INTERNAL ERROR: Bad magic at options.c, line 1059
Aborted (core dumped)

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Richard Laager (rlaager)]

I hit this bug. The analysis here appears correct to me. PIDFILE is a static string (via a preprocessor define). The suggested fix of calling str_dup() sounds correct.

Adding this to the top of a stunnel config file is a work-around:
pid = /var/run/stunnel4.pid

[Launchpad Janitor (janitor)]

This bug was fixed in the package stunnel4 - 3:5.56+dfsg-10

---------------
stunnel4 (3:5.56+dfsg-10) unstable; urgency=medium

  * Allocate a new memory buffer for the pidfile path in
    the 04-restore-pidfile-default patch, since it may be freed when
    stunnel is reconfigured, e.g. on a SIGHUP.
    Closes: #987265; LP: #1901784

 -- Peter Pentchev <email address hidden> Wed, 21 Apr 2021 09:12:28 +0300