Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 29 Mar 2023 21:49:26 -0700
From:      John Baldwin <jhb@freebsd.org>
To:        Mark Johnston <markj@freebsd.org>
Cc:        src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org
Subject:   Re: git: 68ca8363c7a1 - main - libc: Use secure_getenv(3) where appropriate
Message-ID:  <933D3669-FAC3-4D3C-937E-93F4F7387D5F@freebsd.org>
In-Reply-To: <202303271256.32RCuV1K007625@gitrepo.freebsd.org>
References:  <202303271256.32RCuV1K007625@gitrepo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
This looks to have broken cross-builds on MacOS as the MacOS jobs on GitHub a=
re now failing due to missing the prototype for this function building shims=
 for btree from libc.

--=20
John Baldwin

> On Mar 27, 2023, at 05:56, Mark Johnston <markj@freebsd.org> wrote:
>=20
> =EF=BB=BFThe branch main has been updated by markj:
>=20
> URL: https://cgit.FreeBSD.org/src/commit/?id=3D68ca8363c7a19d5351dc2b10568=
cbf2403e07e33
>=20
> commit 68ca8363c7a19d5351dc2b10568cbf2403e07e33
> Author:     Mark Johnston <markj@FreeBSD.org>
> AuthorDate: 2023-03-27 12:55:01 +0000
> Commit:     Mark Johnston <markj@FreeBSD.org>
> CommitDate: 2023-03-27 12:56:22 +0000
>=20
>    libc: Use secure_getenv(3) where appropriate
>=20
>    No functional change intended.
>=20
>    Reviewed by:    mjg, imp, kib
>    Differential Revision:  https://reviews.freebsd.org/D39278
> ---
> lib/libc/db/btree/bt_open.c    |  5 ++---
> lib/libc/db/hash/hash_page.c   |  5 ++---
> lib/libc/gen/fstab.c           |  8 ++------
> lib/libc/gen/glob-compat11.c   |  3 +--
> lib/libc/gen/glob.c            |  3 +--
> lib/libc/iconv/citrus_iconv.c  |  5 +++--
> lib/libc/iconv/citrus_module.c |  4 ++--
> lib/libc/locale/setlocale.c    |  4 ++--
> lib/libc/net/hesiod.c          | 10 ++--------
> lib/libc/net/rcmd.c            |  2 +-
> lib/libc/nls/msgcat.c          |  2 +-
> lib/libc/posix1e/mac.c         |  5 ++---
> lib/libc/resolv/res_init.c     |  2 +-
> lib/libc/resolv/res_query.c    |  4 +---
> lib/libc/stdio/tempnam.c       |  2 +-
> lib/libc/stdio/tmpfile.c       |  4 +---
> 16 files changed, 25 insertions(+), 43 deletions(-)
>=20
> diff --git a/lib/libc/db/btree/bt_open.c b/lib/libc/db/btree/bt_open.c
> index ce3b8a1ecf1b..06bbd39f1842 100644
> --- a/lib/libc/db/btree/bt_open.c
> +++ b/lib/libc/db/btree/bt_open.c
> @@ -391,11 +391,10 @@ tmp(void)
> {
>    sigset_t set, oset;
>    int fd, len;
> -    char *envtmp =3D NULL;
> +    char *envtmp;
>    char path[MAXPATHLEN];
>=20
> -    if (issetugid() =3D=3D 0)
> -        envtmp =3D getenv("TMPDIR");
> +    envtmp =3D secure_getenv("TMPDIR");
>    len =3D snprintf(path,
>        sizeof(path), "%s/bt.XXXXXXXXXX", envtmp ? envtmp : "/tmp");
>    if (len < 0 || len >=3D (int)sizeof(path)) {
> diff --git a/lib/libc/db/hash/hash_page.c b/lib/libc/db/hash/hash_page.c
> index fba854b51f33..afcda6321133 100644
> --- a/lib/libc/db/hash/hash_page.c
> +++ b/lib/libc/db/hash/hash_page.c
> @@ -855,11 +855,10 @@ open_temp(HTAB *hashp)
> {
>    sigset_t set, oset;
>    int len;
> -    char *envtmp =3D NULL;
> +    char *envtmp;
>    char path[MAXPATHLEN];
>=20
> -    if (issetugid() =3D=3D 0)
> -        envtmp =3D getenv("TMPDIR");
> +    envtmp =3D secure_getenv("TMPDIR");
>    len =3D snprintf(path,
>        sizeof(path), "%s/_hash.XXXXXX", envtmp ? envtmp : "/tmp");
>    if (len < 0 || len >=3D (int)sizeof(path)) {
> diff --git a/lib/libc/gen/fstab.c b/lib/libc/gen/fstab.c
> index 3813202afb15..718373931757 100644
> --- a/lib/libc/gen/fstab.c
> +++ b/lib/libc/gen/fstab.c
> @@ -259,12 +259,8 @@ setfsent(void)
>        LineNo =3D 0;
>        return (1);
>    }
> -    if (fsp_set =3D=3D 0) {
> -        if (issetugid())
> -            setfstab(NULL);
> -        else
> -            setfstab(getenv("PATH_FSTAB"));
> -    }
> +    if (fsp_set =3D=3D 0)
> +        setfstab(secure_getenv("PATH_FSTAB"));
>    if ((_fs_fp =3D fopen(path_fstab, "re")) !=3D NULL) {
>        LineNo =3D 0;
>        return (1);
> diff --git a/lib/libc/gen/glob-compat11.c b/lib/libc/gen/glob-compat11.c
> index 76a4553c922c..26dc9db9ff29 100644
> --- a/lib/libc/gen/glob-compat11.c
> +++ b/lib/libc/gen/glob-compat11.c
> @@ -422,8 +422,7 @@ globtilde(const Char *pattern, Char *patbuf, size_t pa=
tbuf_len, glob11_t *pglob)
>         * we're not running setuid or setgid) and then trying
>         * the password file
>         */
> -        if (issetugid() !=3D 0 ||
> -            (h =3D getenv("HOME")) =3D=3D NULL) {
> +        if ((h =3D secure_getenv("HOME")) =3D=3D NULL) {
>            if (((h =3D getlogin()) !=3D NULL &&
>                 (pwd =3D getpwnam(h)) !=3D NULL) ||
>                (pwd =3D getpwuid(getuid())) !=3D NULL)
> diff --git a/lib/libc/gen/glob.c b/lib/libc/gen/glob.c
> index 2e8bf6310641..43dd77df8119 100644
> --- a/lib/libc/gen/glob.c
> +++ b/lib/libc/gen/glob.c
> @@ -453,8 +453,7 @@ globtilde(const Char *pattern, Char *patbuf, size_t pa=
tbuf_len, glob_t *pglob)
>         * we're not running setuid or setgid) and then trying
>         * the password file
>         */
> -        if (issetugid() !=3D 0 ||
> -            (h =3D getenv("HOME")) =3D=3D NULL) {
> +        if ((h =3D secure_getenv("HOME")) =3D=3D NULL) {
>            if (((h =3D getlogin()) !=3D NULL &&
>                 (pwd =3D getpwnam(h)) !=3D NULL) ||
>                (pwd =3D getpwuid(getuid())) !=3D NULL)
> diff --git a/lib/libc/iconv/citrus_iconv.c b/lib/libc/iconv/citrus_iconv.c=

> index 88dfc2deca33..27f88c6a47ab 100644
> --- a/lib/libc/iconv/citrus_iconv.c
> +++ b/lib/libc/iconv/citrus_iconv.c
> @@ -81,8 +81,9 @@ init_cache(void)
>        _CITRUS_HASH_INIT(&shared_pool, CI_HASH_SIZE);
>        TAILQ_INIT(&shared_unused);
>        shared_max_reuse =3D -1;
> -        if (!issetugid() && getenv(CI_ENV_MAX_REUSE))
> -            shared_max_reuse =3D atoi(getenv(CI_ENV_MAX_REUSE));
> +        if (secure_getenv(CI_ENV_MAX_REUSE) !=3D NULL)
> +            shared_max_reuse =3D
> +                atoi(secure_getenv(CI_ENV_MAX_REUSE));
>        if (shared_max_reuse < 0)
>            shared_max_reuse =3D CI_INITIAL_MAX_REUSE;
>        isinit =3D true;
> diff --git a/lib/libc/iconv/citrus_module.c b/lib/libc/iconv/citrus_module=
.c
> index bd173b41bb04..76db1bc7df9c 100644
> --- a/lib/libc/iconv/citrus_module.c
> +++ b/lib/libc/iconv/citrus_module.c
> @@ -282,8 +282,8 @@ _citrus_load_module(_citrus_module_t *rhandle, const c=
har *encname)
>    int maj, min;
>=20
>    if (_pathI18nModule =3D=3D NULL) {
> -        p =3D getenv("PATH_I18NMODULE");
> -        if (p !=3D NULL && !issetugid()) {
> +        p =3D secure_getenv("PATH_I18NMODULE");
> +        if (p !=3D NULL) {
>            _pathI18nModule =3D strdup(p);
>            if (_pathI18nModule =3D=3D NULL)
>                return (ENOMEM);
> diff --git a/lib/libc/locale/setlocale.c b/lib/libc/locale/setlocale.c
> index e0ba66e0e35a..bb60418f3583 100644
> --- a/lib/libc/locale/setlocale.c
> +++ b/lib/libc/locale/setlocale.c
> @@ -312,9 +312,9 @@ int
> __detect_path_locale(void)
> {
>    if (_PathLocale =3D=3D NULL) {
> -        char *p =3D getenv("PATH_LOCALE");
> +        char *p =3D secure_getenv("PATH_LOCALE");
>=20
> -        if (p !=3D NULL && !issetugid()) {
> +        if (p !=3D NULL) {
>            if (strlen(p) + 1/*"/"*/ + ENCODING_LEN +
>                1/*"/"*/ + CATEGORY_LEN >=3D PATH_MAX)
>                return (ENAMETOOLONG);
> diff --git a/lib/libc/net/hesiod.c b/lib/libc/net/hesiod.c
> index 0966b6d7ef91..f456e76316a1 100644
> --- a/lib/libc/net/hesiod.c
> +++ b/lib/libc/net/hesiod.c
> @@ -92,10 +92,7 @@ hesiod_init(context)
>    ctx =3D malloc(sizeof(struct hesiod_p));
>    if (ctx) {
>        *context =3D ctx;
> -        if (!issetugid())
> -            configname =3D getenv("HESIOD_CONFIG");
> -        else
> -            configname =3D NULL;
> +        configname =3D secure_getenv("HESIOD_CONFIG");
>        if (!configname)
>            configname =3D _PATH_HESIOD_CONF;
>        if (read_config_file(ctx, configname) >=3D 0) {
> @@ -103,10 +100,7 @@ hesiod_init(context)
>             * The default rhs can be overridden by an
>             * environment variable.
>             */
> -            if (!issetugid())
> -                p =3D getenv("HES_DOMAIN");
> -            else
> -                p =3D NULL;
> +            p =3D secure_getenv("HES_DOMAIN");
>            if (p) {
>                if (ctx->rhs)
>                    free(ctx->rhs);
> diff --git a/lib/libc/net/rcmd.c b/lib/libc/net/rcmd.c
> index e8b4ffd356c4..2a6edd66440c 100644
> --- a/lib/libc/net/rcmd.c
> +++ b/lib/libc/net/rcmd.c
> @@ -97,7 +97,7 @@ rcmd_af(char **ahost, int rport, const char *locuser, co=
nst char *remuser,
>    static char canonnamebuf[MAXDNAME];    /* is it proper here? */
>=20
>    /* call rcmdsh() with specified remote shell if appropriate. */
> -    if (!issetugid() && (p =3D getenv("RSH"))) {
> +    if ((p =3D secure_getenv("RSH")) !=3D NULL) {
>        struct servent *sp =3D getservbyname("shell", "tcp");
>=20
>        if (sp && sp->s_port =3D=3D rport)
> diff --git a/lib/libc/nls/msgcat.c b/lib/libc/nls/msgcat.c
> index f27bf7918b88..7f687258e5c3 100644
> --- a/lib/libc/nls/msgcat.c
> +++ b/lib/libc/nls/msgcat.c
> @@ -196,7 +196,7 @@ __catopen_l(const char *name, int type, locale_t local=
e)
>        pcode =3D cptr;
>    }
>=20
> -    if ((nlspath =3D getenv("NLSPATH")) =3D=3D NULL || issetugid())
> +    if ((nlspath =3D secure_getenv("NLSPATH")) =3D=3D NULL)
>        nlspath =3D _DEFAULT_NLS_PATH;
>=20
>    if ((base =3D cptr =3D strdup(nlspath)) =3D=3D NULL) {
> diff --git a/lib/libc/posix1e/mac.c b/lib/libc/posix1e/mac.c
> index a8e0abe7afff..7747b62b7c72 100644
> --- a/lib/libc/posix1e/mac.c
> +++ b/lib/libc/posix1e/mac.c
> @@ -177,9 +177,8 @@ mac_init_internal(int ignore_errors)
>=20
>    LIST_INIT(&label_default_head);
>=20
> -    if (!issetugid() && getenv("MAC_CONFFILE") !=3D NULL)
> -        filename =3D getenv("MAC_CONFFILE");
> -    else
> +    filename =3D secure_getenv("MAC_CONFFILE");
> +    if (filename =3D=3D NULL)
>        filename =3D MAC_CONFFILE;
>    file =3D fopen(filename, "re");
>    if (file =3D=3D NULL)
> diff --git a/lib/libc/resolv/res_init.c b/lib/libc/resolv/res_init.c
> index 274ffbf999d6..40d3373e813d 100644
> --- a/lib/libc/resolv/res_init.c
> +++ b/lib/libc/resolv/res_init.c
> @@ -277,7 +277,7 @@ __res_vinit(res_state statp, int preinit) {
> #endif    /* SOLARIS2 */
>=20
>    /* Allow user to override the local domain definition */
> -    if (issetugid() =3D=3D 0 && (cp =3D getenv("LOCALDOMAIN")) !=3D NULL)=
 {
> +    if ((cp =3D secure_getenv("LOCALDOMAIN")) !=3D NULL) {
>        (void)strncpy(statp->defdname, cp, sizeof(statp->defdname) - 1);
>        statp->defdname[sizeof(statp->defdname) - 1] =3D '\0';
>        haveenv++;
> diff --git a/lib/libc/resolv/res_query.c b/lib/libc/resolv/res_query.c
> index 8270e26ecdfb..10ac46ced8af 100644
> --- a/lib/libc/resolv/res_query.c
> +++ b/lib/libc/resolv/res_query.c
> @@ -457,9 +457,7 @@ res_hostalias(const res_state statp, const char *name,=
 char *dst, size_t siz) {
>=20
>    if (statp->options & RES_NOALIASES)
>        return (NULL);
> -    if (issetugid())
> -        return (NULL);
> -    file =3D getenv("HOSTALIASES");
> +    file =3D secure_getenv("HOSTALIASES");
>    if (file =3D=3D NULL || (fp =3D fopen(file, "re")) =3D=3D NULL)
>        return (NULL);
>    setbuf(fp, NULL);
> diff --git a/lib/libc/stdio/tempnam.c b/lib/libc/stdio/tempnam.c
> index 2d7bd90e08a4..4a720fd4c1cb 100644
> --- a/lib/libc/stdio/tempnam.c
> +++ b/lib/libc/stdio/tempnam.c
> @@ -60,7 +60,7 @@ tempnam(const char *dir, const char *pfx)
>    if (!pfx)
>        pfx =3D "tmp.";
>=20
> -    if (issetugid() =3D=3D 0 && (f =3D getenv("TMPDIR"))) {
> +    if ((f =3D secure_getenv("TMPDIR")) !=3D NULL) {
>        (void)snprintf(name, MAXPATHLEN, "%s%s%sXXXXXX", f,
>            *(f + strlen(f) - 1) =3D=3D '/'? "": "/", pfx);
>        if ((f =3D _mktemp(name)))
> diff --git a/lib/libc/stdio/tmpfile.c b/lib/libc/stdio/tmpfile.c
> index e5ee1be2884e..aedaab6e1262 100644
> --- a/lib/libc/stdio/tmpfile.c
> +++ b/lib/libc/stdio/tmpfile.c
> @@ -60,9 +60,7 @@ tmpfile(void)
>    char *buf;
>    const char *tmpdir;
>=20
> -    tmpdir =3D NULL;
> -    if (issetugid() =3D=3D 0)
> -        tmpdir =3D getenv("TMPDIR");
> +    tmpdir =3D secure_getenv("TMPDIR");
>    if (tmpdir =3D=3D NULL)
>        tmpdir =3D _PATH_TMP;
>=20



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?933D3669-FAC3-4D3C-937E-93F4F7387D5F>