[Bf-committers] [PATCH] Patch to storage.c to avoid crash on long userids

[Rob M]

Hi,

The patch is bad style. You should not pass literal sizes to strncpy - what
happens when someone changes the size of the array?
Passing sizeof(files[num].owner) is better (but doesn't allow easy
translation from 8bit chars to wide chars).
Would it not also be better to increase the size of the owner field as well
as not overflowing the array?

Regards,

Rob

> -----Original Message-----
> From: bf-committers-bounces at blender.org [mailto:bf-committers-
> bounces at blender.org] On Behalf Of Jochen Schmitt
> Sent: Sunday, March 28, 2010 6:40 PM
> To: bf-committers at blender.org
> Subject: [Bf-committers] [PATCH] Patch to storage.c to avoid crash on
> long userids
> 
> Hallo,
> 
> on
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=572186
> 
> an user complains a crash which seems to occured, if he use
> an userid which contains more the 15 characters. I have found
> a line in the file storage.c where an userid which will be returned
> by the getpwent() function. This userid will been copied into an array
> which is declared as char [16]. Unfortunately on Fedora you can create
> an userid which will be contains more then 16 characters. So I have to
> replace the strcpy() function to strncpy() to make sure, that not more
> then
> 16 characters will been copied even if the userid should be have more
> the 16 characters.
> 
> It may be nice, if anyone can do a review of this patch for inclussion
> into
> the main blender development branch.
> 
> Best Regards:
> 
> Jochen Schmitt
> 
> 
> diff -up blender/source/blender/blenlib/intern/storage.c.uid
> blender/source/blender/blenlib/intern/storage.c
> --- blender/source/blender/blenlib/intern/storage.c.uid 2010-03-28
> 19:18:49.511022187 +0200
> +++ blender/source/blender/blenlib/intern/storage.c     2010-03-28
> 19:20:37.773022342 +0200
> @@ -331,7 +331,8 @@ void BLI_adddirstrings()
>                         struct passwd *pwuser;
>                         pwuser = getpwuid(files[num].s.st_uid);
>                         if ( pwuser ) {
> -                       strcpy(files[num].owner, pwuser->pw_name);
> +                         strncpy(files[num].owner, pwuser->pw_name,
> 15);
> +                          files[num].owner[15] = '\0';
>                         } else {
>                                 sprintf(files[num].owner, "%d",
> files[num].s.st_uid);
>                         }
> ~
> ~