Page MenuHomeFreeBSD

sysutils/lttng-tools: fix obtaining HOST_NAME_MAX
ClosedPublic

Authored by a.wolk_fudosecurity.com on Apr 30 2021, 3:21 PM.
Referenced Files
Unknown Object (File)
Tue, Jan 10, 10:11 PM
Unknown Object (File)
Dec 26 2022, 8:25 PM
Unknown Object (File)
Dec 23 2022, 8:19 AM
Unknown Object (File)
Dec 21 2022, 3:08 AM
Unknown Object (File)
Dec 8 2022, 12:44 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 sysutils/lttng-tools where several patches attempt to
use the sysconf(3) API but then accidentally introduce VLA.

$ ag HOST_NAME
files/patch-src_bin_lttng-sessiond_session.h
10:     char hostname[HOST_NAME_MAX]; /* Local hostname. */

files/patch-src_bin_lttng-sessiond_ust-metadata.c
24:+    const size_t HOST_NAME_MAX = sysconf(_SC_HOST_NAME_MAX);
26:     char hostname[HOST_NAME_MAX];

files/patch-src_bin_lttng_commands_view.c
8:+     const size_t HOST_NAME_MAX = sysconf(_SC_HOST_NAME_MAX);
10:     char hostname[HOST_NAME_MAX];

files/patch-src_bin_lttng-sessiond_consumer.c
8:+     const size_t HOST_NAME_MAX = sysconf(_SC_HOST_NAME_MAX);
10:     char hostname[HOST_NAME_MAX];

Hostname now becomes a variable length array on the stack.

The proper non-intrusive fix for this codebase is to use _POSIX_HOST_NAME_MAX.
This code is full of doubtful uses of hostname / gethostname and accounting.
Including never accounting for the fact that HOST_NAME_MAX is not including
the terminating null so when defining hostname[MAX] an additional byte is not
added for the null. Further, the code over guards itself by placing \0 at MAX-1
at various places in the code potentially cutting the returned hostname short.

Additionally, another patch adds an ifdefed FreeBSD 256 hardcoded
(so _POSIX_HOST_NAME_MAX + 1 for the terimnating null).

struct ltt_session {                                                            
       char name[NAME_MAX];                                                    
#if defined(__FreeBSD__)                                                                                                                                   
       char hostname[256]; /* Local hostname. */                               
#else                                                                           
       char hostname[HOST_NAME_MAX]; /* Local hostname. */                     
#endif

I suggest switching to _POSIX_HOST_NAME_MAX and also changing the 256 to just the
posix max as no other code accounts for the additional null byte this way (HOST_NAME_MAX
on other platform also doesn't contain the null byte). This avoids introducing VLA
and makes the null byte accounting match with what upstream does.

Suggested commit message:

commit 89756a5bf63faac1552b4cc9f917dd9a18ae458a (HEAD -> main)
Author: Adam Wolk <a.wolk@fudosecurity.com>
Date:   Fri Apr 30 17:16:05 2021 +0200

    sysutils/lttng-tools: fix obtaining HOST_NAME_MAX
    
    Using sysconf(3) API lead to accidental introduction of variable length arrays
    (VLA) in the port. Additionally one patch hardcoded 256 as the HOST_NAME_MAX
    even though the code doesn't expect an additional byte for the terminating NULL
    byte in the struct definition.
    
    Fall back to using _POSIX_HOST_NAME_MAX as the remaining code is not ready for
    introducing sysconf(3) as a patch.
    
    Remove #ifdef FreeBSD from our patches.
    
    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

jbeich added a subscriber: jbeich.

Looks OK from ports/ POV and this port doesn't have a maintainer. I remember deciding against using _SC_HOST_NAME_MAX due to being too error-prone in one of my ports.

This revision is now accepted and ready to land.Apr 30 2021, 11:23 PM