Re: [musl] [PATCH] musl: lutimes: Add checks for input parameters

[Rich Felker <dalias@libc.org>]

On Sun, Mar 01, 2020 at 02:57:30PM +0800, Liu Jie wrote:
> For the input parameter struct timeval tv, need to
> determine whether it is invalid inputs.
> 
> Signed-off-by: Liu Jie <liujie1@huawei.com>
> ---
>  src/legacy/lutimes.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/src/legacy/lutimes.c b/src/legacy/lutimes.c
> index 2e5502d1..6e7e1be3 100644
> --- a/src/legacy/lutimes.c
> +++ b/src/legacy/lutimes.c
> @@ -2,13 +2,22 @@
>  #include <sys/stat.h>
>  #include <sys/time.h>
>  #include <fcntl.h>
> +#include <stdio.h>
> +#include <errno.h>
>  
>  int lutimes(const char *filename, const struct timeval tv[2])
>  {
>  	struct timespec times[2];
> -	times[0].tv_sec  = tv[0].tv_sec;
> -	times[0].tv_nsec = tv[0].tv_usec * 1000;
> -	times[1].tv_sec  = tv[1].tv_sec;
> -	times[1].tv_nsec = tv[1].tv_usec * 1000;
> -	return utimensat(AT_FDCWD, filename, times, AT_SYMLINK_NOFOLLOW);
> +	if (tv != NULL) {
> +		if (tv[0].tv_sec < 0 || tv[0].tv_usec < 0 ||
> +		    tv[1].tv_sec < 0 || tv[1].tv_usec < 0) {
> +			errno = EINVAL;
> +			return -1;
> +		}
> +		times[0].tv_sec  = tv[0].tv_sec;
> +		times[0].tv_nsec = tv[0].tv_usec * 1000;
> +		times[1].tv_sec  = tv[1].tv_sec;
> +		times[1].tv_nsec = tv[1].tv_usec * 1000;
> +	}
> +	return utimensat(AT_FDCWD, filename, tv ? times : NULL, AT_SYMLINK_NOFOLLOW);
>  }
> -- 
> 2.17.1

This patch causes uninitialized timespecs to be used if a null pointer
is passed, silently corrupting data. If there is any historical
documented precedent for this function accepting a null pointer and
doing something meaningful, then the patch needs to do whatever that
meaningful thing is rather than usign uninitialized data. If not, the
preferred behavior is the current behavior: to crash so that the usage
error is caught.

Rich