Re: [PATCH] fsck: use xasprintf to avoid buffer overruns with an insane fs type

[Peter Cordes <peter@cordes.ca>]

On Thu, Feb 15, 2018 at 03:05:08PM -0500, Theodore Ts'o wrote:
> This prevents a crash when running the command:
> 
> fsck -t AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA /dev/sda
> 
> Reported-by: Hornseth_Brenan@bah.com
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> ---
>  disk-utils/fsck.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/disk-utils/fsck.c b/disk-utils/fsck.c
> index 58fd8ac59..8a07bc272 100644
> --- a/disk-utils/fsck.c
> +++ b/disk-utils/fsck.c
> @@ -544,20 +544,20 @@ static char *find_fsck(const char *type)
>  {
>  	char *s;
>  	const char *tpl;
> -	static char prog[256];
> +	static char *prog = NULL;

 You're allocating / freeing every time it's used, so it shouldn't be
static anymore.

It might be easier to just use snprintf to truncate long strings,
instead of introducing dynamic allocation which requires explicit
freeing.  OTOH xasprintf makes it re-entrant / thread-safe, at the
cost of forcing the caller to care about memory management.  (And at
the cost of efficiency: prog is allocated / freed inside the loop.)

>  	char *p = xstrdup(fsck_path);
>  
>  	/* Are we looking for a program or just a type? */
>  	tpl = (strncmp(type, "fsck.", 5) ? "%s/fsck.%s" : "%s/%s");
>  
>  	for(s = strtok(p, ":"); s; s = strtok(NULL, ":")) {
> -		sprintf(prog, tpl, s, type);
> +		xasprintf(&prog, tpl, s, type);
>  		if (access(prog, X_OK) == 0)
>  			break;
> +		free(prog); prog = NULL;
>  	}
>  	free(p);
> -
> -	return(s ? prog : NULL);
> +	return(prog);
>  }


-- 
#define X(x,y) x##y
Peter Cordes ;  e-mail: X(peter@cor , des.ca)

"The gods confound the man who first found out how to distinguish the hours!
 Confound him, too, who in this place set up a sundial, to cut and hack
 my day so wretchedly into small pieces!" -- Plautus, 200 BC