kernel32: Avoid two potential buffer overflows of cStr in create_hardware_branch.

Sebastian Lackner sebastian at fds-team.de
Tue Oct 11 06:44:52 CDT 2016


On 09.10.2016 22:30, Gerald Pfeifer wrote:
> On Sun, 9 Oct 2016, Sebastian Lackner wrote:
>> On 08.10.2016 22:31, Gerald Pfeifer wrote:
>>> +    char cStr[sizeof(dent->d_name)+sizeof(procname_ide_media)],
>> http://man7.org/linux/man-pages/man3/readdir.3.html says:
>>
>> """The standard also notes that the use of sizeof(d_name) is
>> incorrect; use strlen(d_name) instead.  (On some systems, this field
>> is defined as char d_name[1]!)"""
>>
>> This means your solution will not work on all systems.
> 
> Interesting, thanks!  Does this happen on any system we care about? 
> 
> I checked a current GNU/Linux system and FreeBSD, and on both 
> sizeof(d_name) works.  Still, good to make this more portable,
> so would 
> 
>   char cStr[NAME_MAX+sizeof(procname_ide_media)]
> 
> work?
> 
> According to that man page
> 
>   Warning: applications should avoid any dependence on the  size  of  the
>   d_name  field.  POSIX defines it as char d_name[], a character array of
>   unspecified size, with at most NAME_MAX characters preceding the termi-
>   nating null byte ('\0').
> 
> so we should be fine, shouldn't we?
> 
> Gerald
> 

NAME_MAX is only used at a few places, so not sure if it works on all platforms.
The best method is usually to allocate a buffer dynamically, but not sure if its
worth the effort for such old legacy code. Adding a length check at the beginning
is probably easier.

Btw, please also note that cUnixDeviceName is also affected by potential buffer
overflow issues.

Regards,
Sebastian




More information about the wine-devel mailing list