Page MenuHomeFreeBSD

net/tigervnc-server: fix obtaining HOST_NAME_MAX
ClosedPublic

Authored by a.wolk_fudosecurity.com on Apr 30 2021, 1:45 PM.
Referenced Files
Unknown Object (File)
Sat, Mar 9, 5:29 PM
Unknown Object (File)
Sat, Mar 9, 5:29 PM
Unknown Object (File)
Sat, Mar 9, 5:29 PM
Unknown Object (File)
Sat, Mar 9, 5:17 PM
Unknown Object (File)
Jan 16 2024, 6:24 PM
Unknown Object (File)
Dec 20 2023, 7:28 AM
Unknown Object (File)
Dec 20 2023, 4:16 AM
Unknown Object (File)
Dec 13 2023, 8:40 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 net/tigervnc-server where a patch was added:

https://svnweb.freebsd.org/ports/head/net/tigervnc-server/files/patch-unix_vncserver_vncsession.c?revision=550391&view=markup

diff --git unix/vncserver/vncsession.c unix/vncserver/vncsession.c
index 3e0c98f0..cdac9d1a 100644
--- unix/vncserver/vncsession.c
+++ unix/vncserver/vncsession.c
@@ -339,7 +339,7 @@ static void
 redir_stdio(const char *homedir, const char *display)
 {
     int fd;
-    char hostname[HOST_NAME_MAX+1];
+    char hostname[_SC_HOST_NAME_MAX+1];
     char logfile[PATH_MAX];
 
     fd = open("/dev/null", O_RDONLY);

Instead of using the maximal allowed host name we use an accidental value '72'.

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

Diff Detail

Repository
R11 FreeBSD ports repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

This is the commit message that phrabricator silently removed:

commit d4ba406c38f5be54199a46822282cd33e2630645 (HEAD -> main)
Author: Adam Wolk <a.wolk@fudosecurity.com>
Date:   Fri Apr 30 15:24:46 2021 +0200

    net/tigervnc-server: fix obtaining HOST_NAME_MAX
    
    Mistaking sysconf(3) API lead to using a random value (72) as the HOST_NAME_MAX
    instead of actual HOST_NAME_MAX.
    
    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
jbeich added a subscriber: jbeich.

This port has a maintainer. Assigning as reviewer.

This revision is now accepted and ready to land.May 3 2021, 2:41 PM

I have already approved this. Feel free to commit with commit message Approved by: meta (maintainer).

@meta Unfortunately I don't have a commit bit. Who can I ask to commit the change?

@meta Unfortunately I don't have a commit bit. Who can I ask to commit the change?

Ah sorry, I thought submitter is comitter. I'll commit it.

Committed, thanks for the patience!