[vlc-devel] [PATCH] Fix Hurd build

[Denis Charmet]

On 2016-04-27 13:21, Samuel Thibault wrote:
> Denis Charmet, on Wed 27 Apr 2016 12:10:54 +0200, wrote:
>> On 2016-04-26 21:14, Samuel Thibault wrote:
>>> Rémi Denis-Courmont, on Tue 26 Apr 2016 22:12:04 +0300, wrote:
>>>> I mean the following sprintf(). Maybe there was a nonobvious way to
>>>> prevent
>>>> overflow, but I don´t see it.
>>> 
>>> Ok. The "idea" behind PATH_MAX is that it's supposed to be the 
>>> maximum
>>> size you'd want to pass with prefix and filename. But yes, that 
>>> won't
>>> prevent anybody from actually passing bigger filenames, and so the
>>> second patch I sent, which uses malloc, just avoids the issue 
>>> altogether.
>> 
>> I think the point of Rémi was: use snprintf and not sprintf 
>> regardless of
>> your memory zone of allocation.
> 
> Then this?

Nope let the stack buffer with snprintf

> 
>> From 49ea01e4bc56d98a0c83179aa41bd1a3d35f4205 Mon Sep 17 00:00:00 
>> 2001
> From: Samuel Thibault <samuel.thibault at ens-lyon.org>
> Date: Tue, 26 Apr 2016 18:22:41 +0000
> Subject: [PATCH] Fix hurd build
> 
> theme_loader.cpp contains an unconditional use of PATH_MAX,
> which is not defined on GNU/Hurd to avoid imposing build-time
> limits. This change replaces its use with dynamic allocation of the
> required size.
> ---
>  modules/gui/skins2/src/theme_loader.cpp | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/modules/gui/skins2/src/theme_loader.cpp
> b/modules/gui/skins2/src/theme_loader.cpp
> index 1ec1dfc..6e7cb5f 100644
> --- a/modules/gui/skins2/src/theme_loader.cpp
> +++ b/modules/gui/skins2/src/theme_loader.cpp
> @@ -549,7 +549,7 @@ int tar_extract_all( TAR *t, char *prefix )
>      union tar_buffer buffer;
>      int   len, err, getheader = 1, remaining = 0;
>      FILE  *outfile = NULL;
> -    char  fname[BLOCKSIZE + PATH_MAX];
> +    char  *fname = NULL;
> 
>      while( 1 )
>      {
> @@ -566,6 +566,7 @@ int tar_extract_all( TAR *t, char *prefix )
>          if( len != 0 && len != BLOCKSIZE )
>          {
>              fprintf( stderr, "gzread: incomplete block read\n" );
> +            free( fname );
>              return -1;
>          }
> 
> @@ -583,13 +584,16 @@ int tar_extract_all( TAR *t, char *prefix )
>                  break;
>              }
> 
> -            sprintf( fname, "%s/%s", prefix, buffer.header.name );
> +            size_t len = strlen(prefix) + 1 + 
> strlen(buffer.header.name) + 1;

this might integer overflow

> +            fname = (char*) malloc( len );

lack of check

> +            snprintf( fname, len, "%s/%s", prefix, buffer.header.name 
> );
> 
>              /* Check magic value in header */
>              if( strncmp( buffer.header.magic, "GNUtar", 6 ) &&
>                  strncmp( buffer.header.magic, "ustar", 5 ) )
>              {
>                  //fprintf(stderr, "not a tar file\n");
> +                free( fname );
>                  return -1;
>              }
> 
> @@ -659,6 +663,8 @@ int tar_extract_all( TAR *t, char *prefix )
>          }
>      }
> 
> +    free( fname );
> +
>      return 0;
>  }

Regards,
-- 
Denis Charmet - TypX
Le mauvais esprit est un art de vivre