Re: [PATCH][BZ #15670] Bound alloca in __tzfile_read.

["Joseph S. Myers" <joseph at codesourcery dot com>]

On Mon, 14 Oct 2013, Ondrej Bilka wrote:

> This is one of bugs that take longer to read than to fix. There is a
> unbound alloca and obvious limit is PATH_MAX.

You can't assume PATH_MAX exists.  The GNU Hurd has no such hardcoded 
limits.

> 	* time/tzfile.c (__tzfile_read): Bound memory allocated by PATH_MAX.
> 
> diff --git a/misc/error.c b/misc/error.c
> index c8e62cf..a34cbbf 100644
> --- a/misc/error.c
> +++ b/misc/error.c
> @@ -41,6 +41,11 @@
>  # define _(String) String
>  #endif
>  
> +#ifndef PATH_MAX
> +# define PATH_MAX 1024
> +#endif

Apart from defaulting such a value simply being wrong (see the GNU Coding 
Standards - no arbitrary limits in the GNU system), this file has nothing 
to do with the bug in question.

> @@ -157,6 +157,8 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
>        else
>  	tzdir_len = strlen (tzdir);
>        len = strlen (file) + 1;
> +      if (tzdir_len + len > PATH_MAX)
> +	goto ret_free_transitions;

This also doesn't deal with the point in the bug that strlen (either 
strlen) could overflow the "unsigned int" variables.

You need to change both variables to size_t and check __libc_use_alloca to 
determine whether to use alloca or malloc.

-- 
Joseph S. Myers
joseph@codesourcery.com