Page MenuHomeFreeBSD

www/chromium: fix obtaining HOST_NAME_MAX
ClosedPublic

Authored by a.wolk_fudosecurity.com on Apr 30 2021, 7:09 PM.
Referenced Files
Unknown Object (File)
Tue, Jan 3, 7:54 AM
Unknown Object (File)
Dec 28 2022, 1:31 PM
Unknown Object (File)
Dec 23 2022, 8:54 PM
Unknown Object (File)
Dec 19 2022, 3:58 PM
Unknown Object (File)
Dec 19 2022, 3:54 PM
Unknown Object (File)
Dec 19 2022, 3:51 PM
Unknown Object (File)
Dec 2 2022, 10:26 AM
Subscribers

Details

Summary

FreeBSD removed the HOST_NAME_MAX define in an attempt to have donwstream
consumers use the sysconf(3) API. This change has lead to various consumers
erroneously using the new API. Either mistaking _SC_HOST_NAME_MAX, which is
a parameter to sysconf(3) as the replacement of HOST_NAME_MAX or by using
sysconf(_SC_HOST_NAME_MAX) as a compile time constant which in turn can
lead to accidental introduction of variable length arrays (VLA).

The decision has been documented in /usr/include/sys/syslimits.h

/*                                                                              
 * We leave the following values undefined to force applications to either      
 * assume conservative values or call sysconf() to get the current value.       
 *                                                                              
 * HOST_NAME_MAX                                                                
 *                                                                              
 * (We should do this for most of the values currently defined here,            
 * but many programs are not prepared to deal with this yet.)                   
 */

The following snippet of code demonstrates _POSIX_HOST_NAME_MAX which is the
equivalent of HOST_NAME_MAX on most platforms and the confusion between _SC_HOST_NAME_MAX
and sysconf(_SC_HOST_NAME_MAX):

$ cat hostname.c 
#include <unistd.h>
#include <stdio.h>
#include <limits.h>
int
main(int argc, char **argv)
{
        printf("_POSIX_HOST_NAME_MAX=%d\n", _POSIX_HOST_NAME_MAX);
        printf("_SC_HOST_NAME_MAX=%d\n", _SC_HOST_NAME_MAX);
        printf("sysconf(_SC_HOST_NAME_MAX)=%ld\n", sysconf(_SC_HOST_NAME_MAX));
        return 0;
}

Output:

$ ./hostname
_POSIX_HOST_NAME_MAX=255
_SC_HOST_NAME_MAX=72
sysconf(_SC_HOST_NAME_MAX)=255
$ 

By accident _SC_HOST_NAME_MAX has a value large enough to not be spotted in most 'quick tests' people
might make.

This review addresses one of the places of misuse I found. Stay tuned as more will follow.

When committing any of these changes please credit the work as:

Sponsored by: Fudo Security

One of such consumers is port www/chromium where a patch was added:

files/patch-components_sync__device__info_local__device__info__util__linux.cc
7:-  char hostname[HOST_NAME_MAX];
8:-  if (gethostname(hostname, HOST_NAME_MAX) == 0)  // Success.
9:+  int len = sysconf(_SC_HOST_NAME_MAX);
11:+  if (gethostname(hostname, _SC_HOST_NAME_MAX) == 0)  // Success.

The diff goes out of it's way to use sysconf(3) and introduces a variable length
array (VLA). It then fails to use the computed length in call to gethostname passing
in an accidental value '72'.

The proper non-intrusive fix for this codebase is to use _POSIX_HOST_NAME_MAX.

Interestingly, there was a different approach in another path in the same port:

files/patch-components_policy_core_common_cloud_cloud__policy__util.cc
35:+  char hostname[MAXHOSTNAMELEN];
36:+  if (gethostname(hostname, MAXHOSTNAMELEN) == 0)

which uses MAXHOSTNAMELEN instead of HOST_NAME_MAX obtained indirectly (not present
in the patched file) by using sys/params.h in the source tree.

In the spirit of least intrusion into a fragile and important codebase like Chromium I'm
opting for the smallest change of using _POSIX_HOST_NAME_MAX instead of sysconf(3) and
not changing MAXHOSTNAMELEN as that is already a tested change.

Suggested commit message:

commit c17dc6b43ec3781fb2689e0d36c1c1c0b498aabc (HEAD -> main)
Author: Adam Wolk <a.wolk@fudosecurity.com>
Date:   Fri Apr 30 21:04:16 2021 +0200

    www/chromium: fix obtaining HOST_NAME_MAX
    
    Using sysconf(3) API lead to accidental introduction of variable length arrays
    (VLA) in the port. Additionally gethostname(3) has been passed _SC_HOST_NAME_MAX
    incorrectly as the HOST_NAME_MAX length.
    
    Fall back to using _POSIX_HOST_NAME_MAX as the remaining code is not ready for
    introducing sysconf(3) as a patch.
    
    Bump PORTREVISION to rebuild with the new patch.
    
    Sponsored by:   Fudo Security

Diff Detail

Repository
R11 FreeBSD ports repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

This revision is now accepted and ready to land.Dec 14 2021, 6:35 PM
This revision was automatically updated to reflect the committed changes.