Re: [PATCH 1/2] unshare: Fix parsing of id maps

[Karel Zak <kzak@redhat.com>]

On Sat, Jan 15, 2022 at 11:29:25AM -0500, Sean Anderson wrote:
> For whatever reason, mem2strcpy places the nul-terminator at the end of
> the buffer

Yes, it was originally designed for utmp-like strings where rest of
the buffer is filled by nul-terminators. I think we can fix it to make
it usable for normal strings too.

> instead of at the end of the string it copies. This makes it
> completely useless for our purposes, since one would have to add a
> terminator manually to avoid getting garbage. Just use memcpy instead.
> 
> Fixes: ff5dc96eb ("unshare: Add options to map blocks of user/group IDs")
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> Reported-by: Daniel Gerber <dg@atufi.org>
> ---
> 
>  sys-utils/unshare.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/sys-utils/unshare.c b/sys-utils/unshare.c
> index 443358952..889c561ca 100644
> --- a/sys-utils/unshare.c
> +++ b/sys-utils/unshare.c
> @@ -387,8 +387,9 @@ static int uint_to_id(const char *name, size_t sz)
>  {
>  	char buf[UID_BUFSIZ];
>  
> -	mem2strcpy(buf, name, sz, sizeof(buf));
> -	return strtoul_or_err(name, _("could not parse ID"));
> +	memcpy(buf, name, min(sz, sizeof(buf) - 1));
> +	buf[sz] = '\0';
        ^^
What about sz > sizeof(buf)?

Maybe it would be enough to improve mem2strcpy() in include/strutils.h 

 - dest[nmax-1] = '\0';
 + dest[n] = '\0';

 Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com