head 1.1; access; symbols; locks; strict; comment @# @; 1.1 date 2026.09.25.14.20.39; author wiz; state Exp; branches; next ; commitid TmvT2kz6VnKzH0XG; desc @@ 1.1 log @thttpd: fix a couple CVEs Rename patches, add some patches from Debian/FreeBSD, move MESSAGE to README.pkgsrc. Bump PKGREVISION. From Showta Ishizaki in PR 60761. @ text @$NetBSD$ CVE-2007-0158 -- buffer underflow. Five places index with length-1 without checking the length. Three are in expand_symlinks(): - with no_symlink_check (chroot mode) and a path of only slashes, the trailing-slash trim drives checkedlen to 0 and then reads checked[-1]; stat("/") always succeeds, so "/" reaches it; - with an empty path, rest[restlen-1] reads before the buffer; - with a zero-length symlink target in the served tree, readlink() returns 0 and lnk[linklen-1] reads before the stack buffer. The fourth is in auth_check2(): fgets() returns non-NULL for a line that begins with a NUL byte, strlen() is then 0, and line[l-1] reads before the 500-byte stack buffer. The .htpasswd is user-written, so this is reached the same way CVE-2012-5640 is, by requesting a protected directory. The fifth is in thttpd.c: max_connects is fdwatch_get_nfiles() minus SPARE_FDS, with no lower bound, so a file descriptor limit of 10 leaves it at 0. connects[max_connects-1].next_free_connect is then written before the array; malloc(0) returns a pointer, so the allocation does not fail first. With that limit thttpd starts and then refuses every connection (measured: HTTP 200 normally, no answer at all under "ulimit -n 10"), which is what max_connects <= 0 looks like from outside. It now exits with a message instead. The four in libhttpd.c fire under AddressSanitizer on the routines extracted unchanged from 2.29, and are clean once guarded. What other packaging carries, for comparison: site checkedlen restlen readlink auth_check2 FreeBSD ports yes yes no no Debian (2.25b-11) no yes* no no ACME 2.30 (unreleased) - - yes - * Debian drops the trailing-slash trim entirely rather than guarding it. (FreeBSD and Debian also change the PATH_INFO trimming near origfilename[i-1]. That one is not a length-1 underflow -- the stock "i > 0" test already prevents it -- it is a behaviour change, and it is kept separate in patch-libhttpd.c.) ACME's 2.30 changelog lists "off-by-one illegal memory access in expand_symlinks()", which is the readlink() case. --- libhttpd.c.orig +++ libhttpd.c @@@@ -1117,9 +1117,11 @@@@ /* Read it. */ while ( fgets( line, sizeof(line), fp ) != (char*) 0 ) { - /* Nuke newline. */ + /* Nuke newline. A NUL byte in the file leaves strlen() at 0, and + ** line[l-1] would then read before the buffer. + */ l = strlen( line ); - if ( line[l - 1] == '\n' ) + if ( l > 0 && line[l - 1] == '\n' ) line[l - 1] = '\0'; /* Split into user and encrypted password. */ cryp = strchr( line, ':' ); @@@@ -1486,7 +1488,7 @@@@ httpd_realloc_str( &checked, &maxchecked, checkedlen ); (void) strcpy( checked, path ); /* Trim trailing slashes. */ - while ( checked[checkedlen - 1] == '/' ) + while ( checkedlen > 0 && checked[checkedlen - 1] == '/' ) { checked[checkedlen - 1] = '\0'; --checkedlen; @@@@ -1505,7 +1507,7 @@@@ restlen = strlen( path ); httpd_realloc_str( &rest, &maxrest, restlen ); (void) strcpy( rest, path ); - if ( rest[restlen - 1] == '/' ) + if ( restlen > 0 && rest[restlen - 1] == '/' ) rest[--restlen] = '\0'; /* trim trailing slash */ if ( ! tildemapped ) /* Remove any leading slashes. */ @@@@ -1623,7 +1625,9 @@@@ return (char*) 0; } lnk[linklen] = '\0'; - if ( lnk[linklen - 1] == '/' ) + /* An empty symlink target makes readlink() return 0, and + ** lnk[linklen-1] then reads lnk[-1], underflowing the buffer. */ + if ( linklen > 0 && lnk[linklen - 1] == '/' ) lnk[--linklen] = '\0'; /* trim trailing slash */ /* Insert the link contents in front of the rest of the filename. */ --- thttpd.c.orig +++ thttpd.c @@@@ -554,6 +554,17 @@@@ exit( 1 ); } max_connects -= SPARE_FDS; + if ( max_connects <= 0 ) + { + /* With a file descriptor limit this low there is no room for even one + ** connection, and connects[max_connects-1] below would write before + ** the array. malloc(0) returns a pointer, so this is not caught by + ** the allocation failing. + */ + syslog( LOG_CRIT, "file descriptor limit is too low for any connection" ); + (void) fprintf( stderr, "%s: file descriptor limit is too low for any connection\n", argv0 ); + exit( 1 ); + } /* Chroot if requested. */ if ( do_chroot ) @