Re: git: 98f18cd98824 - main - pam_ksu: Move the realm free to end of function

From: Shawn Webb <shawn.webb_at_hardenedbsd.org>
Date: Mon, 16 Jun 2025 18:46:08 UTC
On Mon, Jun 16, 2025 at 06:42:41PM +0000, Cy Schubert wrote:
> The branch main has been updated by cy:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=98f18cd98824acdf1045e74615f2db0219019f0b
> 
> commit 98f18cd98824acdf1045e74615f2db0219019f0b
> Author:     Cy Schubert <cy@FreeBSD.org>
> AuthorDate: 2025-06-16 18:40:51 +0000
> Commit:     Cy Schubert <cy@FreeBSD.org>
> CommitDate: 2025-06-16 18:42:30 +0000
> 
>     pam_ksu: Move the realm free to end of function
>     
>     This avoids a use after free.
>     
>     Noted by:       jhb
> ---
>  lib/libpam/modules/pam_ksu/pam_ksu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/libpam/modules/pam_ksu/pam_ksu.c b/lib/libpam/modules/pam_ksu/pam_ksu.c
> index a6b3f043d3f4..e50c3e387311 100644
> --- a/lib/libpam/modules/pam_ksu/pam_ksu.c
> +++ b/lib/libpam/modules/pam_ksu/pam_ksu.c
> @@ -85,8 +85,6 @@ krb5_make_principal(krb5_context context, krb5_principal principal,
>  		if ((rc = krb5_get_default_realm(context, &temp_realm)))
>  			return (rc);
>  		realm=temp_realm;
> -		if (temp_realm)
> -			free(temp_realm);
>  	}
>  	va_start(ap, realm);
>  	/*
> @@ -99,6 +97,8 @@ krb5_make_principal(krb5_context context, krb5_principal principal,
>  	 */
>  	rc = krb5_build_principal_va(context, principal, strlen(realm), realm, ap);
>  	va_end(ap);
> +	if (temp_realm)
> +		free(temp_realm);

Hey Cy,

I think the call to free can be made unconditional as it's safe to
call free on a NULL pointer (which turns into a no-op).

Thanks,

-- 
Shawn Webb
Cofounder / Security Engineer
HardenedBSD

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