[systemd-bugs] [Bug 90017] New: Improper use of asprintf() in login/pam_systemd.c

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=90017

            Bug ID: 90017
           Summary: Improper use of asprintf() in login/pam_systemd.c
           Product: systemd
           Version: unspecified
          Hardware: Other
                OS: All
            Status: NEW
          Severity: normal
          Priority: medium
         Component: general
          Assignee: systemd-bugs at lists.freedesktop.org
          Reporter: archie.cobbs at gmail.com
        QA Contact: systemd-bugs at lists.freedesktop.org

Reviewing this code:

   
https://github.com/systemd/systemd/blob/master/src/login/pam_systemd.c#L179-190

there appears to be a bug. Here is the code:

    #ifdef ENABLE_KDBUS
        _cleanup_free_ char *s = NULL;
        int r;

        /* skip export if kdbus is not active */
        if (access("/sys/fs/kdbus", F_OK) < 0)
                return PAM_SUCCESS;

        if (asprintf(&s, KERNEL_USER_BUS_ADDRESS_FMT ";"
UNIX_USER_BUS_ADDRESS_FMT, uid, runtime) < 0) {
                pam_syslog(handle, LOG_ERR, "Failed to set bus variable.");
                return PAM_BUF_ERR;
        }

The bug occurs if asprintf() fails. Since "s" is declared _cleanup_free, it
will be automatically free()'d when the function returns (right?).

However, there is no guarantee that "s" will still be equal to NULL at this
point, as the code incorrectly assumes.

Quoting the asprintf() man page:

    RETURN VALUE
       When successful, these functions return the number of bytes printed,
just like sprintf(3).
       If memory allocation wasn't possible, or some other error occurs, these
functions will return -1,
       and the contents of strp is undefined.
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This is only one example; there could be arbitrarily many others in the systemd
codebase (I haven't looked).

In practice, this bug probably never happens, because I doubt that any actual
asprintf() implementations modify "s" on failure. But that's irrelevant to
whether this is a bug.

Personally I think it's stupid that POSIX allows asprintf() to modify *strp on
failure, but whatever, it's too late to fix that now.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/systemd-bugs/attachments/20150413/bea0c314/attachment.html>