Re: git: 6e69612d5df1 - main - pam: Add pam_xdg module

From: Shawn Webb <shawn.webb_at_hardenedbsd.org>
Date: Mon, 26 Feb 2024 20:08:35 UTC
On Mon, Feb 26, 2024 at 06:14:34PM +0000, Shawn Webb wrote:
> On Mon, Feb 26, 2024 at 05:35:57PM +0000, Emmanuel Vadot wrote:
> > The branch main has been updated by manu:
> > 
> > URL: https://cgit.FreeBSD.org/src/commit/?id=6e69612d5df1c1d5bd86990ea4d9a170c030b292
> > 
> > commit 6e69612d5df1c1d5bd86990ea4d9a170c030b292
> > Author:     Emmanuel Vadot <manu@FreeBSD.org>
> > AuthorDate: 2024-02-21 14:51:05 +0000
> > Commit:     Emmanuel Vadot <manu@FreeBSD.org>
> > CommitDate: 2024-02-26 17:34:52 +0000
> > 
> >     pam: Add pam_xdg module
> >     
> >     This is a module to setup the XDG directories and environment variables.
> >     For now the only usage is to have a XDG_RUNTIME_DIR environment setup at
> >     user login.
> >     All other environment variable have a default fallback so no need to export
> >     them in this module.
> >     The directory is created according to the XDG Base directory specification.
> >     
> >     The default base directory is /var/run/xdg/<username> but can be configured
> >     using the runtime_dir=<dir> module option.
> >     
> >     According to the spec the directory *must* not survive a reboot so adding
> >     var_run_enable="YES" to rc.conf is highly recommanded.
> >     
> >     Reviewed by:    des, pauamma (manpages)
> >     Differential Revision:  https://reviews.freebsd.org/D44011
> >     Sponsored by:   Beckhoff Automation GmbH & Co. KG
> > ---
> >  lib/libpam/modules/modules.inc       |   1 +
> >  lib/libpam/modules/pam_xdg/Makefile  |   6 +
> >  lib/libpam/modules/pam_xdg/pam_xdg.8 |  56 +++++++
> >  lib/libpam/modules/pam_xdg/pam_xdg.c | 311 +++++++++++++++++++++++++++++++++++
> >  4 files changed, 374 insertions(+)
> 
> [snip]
> 
> > diff --git a/lib/libpam/modules/pam_xdg/pam_xdg.c b/lib/libpam/modules/pam_xdg/pam_xdg.c
> > new file mode 100644
> > index 000000000000..40012fe463e0
> > --- /dev/null
> > +++ b/lib/libpam/modules/pam_xdg/pam_xdg.c
> > @@ -0,0 +1,311 @@
> > +/*-
> > + * SPDX-License-Identifier: BSD-2-Clause
> > + *
> > + * Copyright (c) 2024 Beckhoff Automation GmbH & Co. KG
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions
> > + * are met:
> > + * 1. Redistributions of source code must retain the above copyright
> > + *    notice, this list of conditions and the following disclaimer.
> > + * 2. Redistributions in binary form must reproduce the above copyright
> > + *    notice, this list of conditions and the following disclaimer in the
> > + *    documentation and/or other materials provided with the distribution.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> > + * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
> > + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> > + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> > + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> > + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> > + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> > + * SUCH DAMAGE.
> > + */
> > +
> > +#include <sys/stat.h>
> > +#include <dirent.h>
> > +#include <errno.h>
> > +#include <fcntl.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <unistd.h>
> > +#include <pwd.h>
> > +
> > +#define	PAM_SM_SESSION
> > +
> > +#include <security/pam_appl.h>
> > +#include <security/pam_modules.h>
> > +#include <security/pam_mod_misc.h>
> > +
> > +#define	BASE_RUNTIME_DIR_PREFIX	"/var/run/xdg"
> > +#define	RUNTIME_DIR_PREFIX	runtime_dir_prefix != NULL ? runtime_dir_prefix : BASE_RUNTIME_DIR_PREFIX
> > +
> > +#define	RUNTIME_DIR_PREFIX_MODE	0711
> > +#define	RUNTIME_DIR_MODE	0700	/* XDG spec */
> > +
> > +#define	XDG_MAX_SESSION		100 /* Arbitrary limit because we need one */
> > +
> > +static int
> > +_pam_xdg_open(pam_handle_t *pamh, int flags __unused,
> > +    int argc __unused, const char *argv[] __unused)
> > +{
> > +	struct passwd *passwd;
> > +	const char *user;
> > +	const char *runtime_dir_prefix;
> > +	struct stat sb;
> > +	char *runtime_dir = NULL;
> > +	char *xdg_session_file;
> > +	int rv, rt_dir_prefix, rt_dir, session_file, i;
> > +
> > +	session_file = -1;
> > +	rt_dir_prefix = -1;
> > +	runtime_dir_prefix = openpam_get_option(pamh, "runtime_dir_prefix");
> > +
> > +	/* Get user info */
> > +	rv = pam_get_item(pamh, PAM_USER, (const void **)&user);
> > +	if (rv != PAM_SUCCESS) {
> > +		PAM_VERBOSE_ERROR("Can't get user information");
> > +		goto out;
> > +	}
> > +	if ((passwd = getpwnam(user)) == NULL) {
> > +		PAM_VERBOSE_ERROR("Can't get user information");
> > +		rv = PAM_SESSION_ERR;
> > +		goto out;
> > +	}
> > +
> > +	/* Open or create the base xdg directory */
> > +	rt_dir_prefix = open(RUNTIME_DIR_PREFIX, O_DIRECTORY | O_NOFOLLOW);
> > +	if (rt_dir_prefix < 0) {
> > +		rt_dir_prefix = mkdir(RUNTIME_DIR_PREFIX, RUNTIME_DIR_PREFIX_MODE);
> > +		if (rt_dir_prefix != 0) {
> > +			PAM_VERBOSE_ERROR("Can't mkdir %s", RUNTIME_DIR_PREFIX);
> > +			rv = PAM_SESSION_ERR;
> > +			goto out;
> > +		}
> > +		rt_dir_prefix = open(RUNTIME_DIR_PREFIX, O_DIRECTORY | O_NOFOLLOW);
> > +	}
> > +
> > +	/* Open or create the user xdg directory */
> > +	rt_dir = openat(rt_dir_prefix, user, O_DIRECTORY | O_NOFOLLOW);
> > +	if (rt_dir < 0) {
> > +		rt_dir = mkdirat(rt_dir_prefix, user, RUNTIME_DIR_MODE);
> > +		if (rt_dir != 0) {
> > +			PAM_VERBOSE_ERROR("mkdir: %s/%s (%d)", RUNTIME_DIR_PREFIX, user, rt_dir);
> > +			rv = PAM_SESSION_ERR;
> > +			goto out;
> > +		}
> > +		rv = fchownat(rt_dir_prefix, user, passwd->pw_uid, passwd->pw_gid, 0);
> > +		if (rv != 0) {
> > +			PAM_VERBOSE_ERROR("fchownat: %s/%s (%d)", RUNTIME_DIR_PREFIX, user, rv);
> > +			rv = unlinkat(rt_dir_prefix, user, AT_REMOVEDIR);
> > +			if (rv == -1)
> > +				PAM_VERBOSE_ERROR("unlinkat: %s/%s (%d)", RUNTIME_DIR_PREFIX, user, errno);
> > +			rv = PAM_SESSION_ERR;
> > +			goto out;
> > +		}
> > +	} else {
> > +		/* Check that the already create dir is correctly owned */
> > +		rv = fstatat(rt_dir_prefix, user, &sb, 0);
> > +		if (rv == -1) {
> > +			PAM_VERBOSE_ERROR("fstatat %s/%s failed (%d)", RUNTIME_DIR_PREFIX, user, errno);
> > +			rv = PAM_SESSION_ERR;
> > +			goto out;
> > +		}
> > +		if (sb.st_uid != passwd->pw_uid ||
> > +		  sb.st_gid != passwd->pw_gid) {
> > +			PAM_VERBOSE_ERROR("%s/%s isn't owned by %d:%d\n", RUNTIME_DIR_PREFIX, user, passwd->pw_uid, passwd->pw_gid);
> > +			rv = PAM_SESSION_ERR;
> > +			goto out;
> > +		}
> > +		/* Test directory mode */
> > +		if ((sb.st_mode & 0x1FF) != RUNTIME_DIR_MODE) {
> > +			PAM_VERBOSE_ERROR("%s/%s have wrong mode\n", RUNTIME_DIR_PREFIX, user);
> > +			rv = PAM_SESSION_ERR;
> > +			goto out;
> > +		}
> > +	}
> > +
> > +	/* Setup the environment variable */
> > +	asprintf(&runtime_dir, "XDG_RUNTIME_DIR=%s/%s", RUNTIME_DIR_PREFIX, user);
> > +	rv = pam_putenv(pamh, runtime_dir);
> > +	if (rv != PAM_SUCCESS) {
> > +		PAM_VERBOSE_ERROR("pam_putenv: failed (%d)", rv);
> > +		rv = PAM_SESSION_ERR;
> > +		goto out;
> > +	}
> > +
> > +	/* Setup the session count file */
> > +	for (i = 0; i < XDG_MAX_SESSION; i++) {
> > +		asprintf(&xdg_session_file, "%s/xdg_session.%d", user, i);
> 
> If asprintf fails, xdg_session_file will be NULL.
> 
> > +		printf("Trying to open %s\n", xdg_session_file);
> > +		session_file = openat(rt_dir_prefix, xdg_session_file, O_CREAT | O_EXCL, RUNTIME_DIR_MODE);
> 
> If xdg_session_file is NULL, there is a NULL pointer dereference
> vulnerability in the above call to openat(2).
> 
> > +		free(xdg_session_file);
> > +		if (session_file >= 0)
> > +			break;
> 

Fixed in HardenedBSD:
https://git.hardenedbsd.org/hardenedbsd/HardenedBSD/-/commit/0b5bf32630b3429e0b5a321e6621a404dc93ecc5

But in that commit, I made a typo, which was subsequently fixed here:
https://git.hardenedbsd.org/hardenedbsd/HardenedBSD/-/commit/865994a5cf20937224bde0662bb1f96ba18e072f

Thanks,

-- 
Shawn Webb
Cofounder / Security Engineer
HardenedBSD

Tor-ified Signal: +1 303-901-1600
https://git.hardenedbsd.org/hardenedbsd/pubkeys/-/raw/master/Shawn_Webb/03A4CBEBB82EA5A67D9F3853FF2E67A277F8E1FA.pub.asc