htest.c was missing the "typedef struct hash hash" that hash.h
requires, and the Makefile linked the top-level ../hash.o which is
built without -DHASH_DEBUG, leaving hash_dump/hash_item_print
undefined. Build a local hash.o with the flag instead. All 16 tests
now compile and pass.
- shovel(): fd2fd() returns FD_NODATA (-1) when a spurious wakeup
meets an empty socket (EAGAIN), but the switch treated -1 like
FD_CNXCLOSED and tore down a healthy connection. Only FD_CNXCLOSED
closes now.
- decrease_forked_connection(): hash_find() can return NULL (stale
SIGCHLD after a fork-limit rejection); dereferencing it crashed.
Guard and log instead.
- hexdump(): sprintf() into a 76-byte stack buffer with a "%06x"
offset that needs 12 chars once the dump passes 16 MB. Enlarge the
buffer and bound all writes with snprintf().
- printsettings(): sprintf() of config-supplied host strings into a
fixed buffer; use snprintf().
- echosrv: recvfrom() into data+prefix_len with size
sizeof(data)-prefix_len lets the terminating NUL write land one
byte past the stack buffer (verified with ASan at udp_echo, and a
>64KB prefix overflows the memcpy). Bound the prefix, reserve one
byte for the terminator, and skip the echo on recvfrom() error
instead of writing data[prefix_len-1].
- msg_int_error used verbose_system_error, leaving the documented
--verbose-int-error option dead. Point it at its own setting.
- Remove "echoѕrv-conf.h" (with Cyrillic ѕ, U+0455): a stale 2020
sslh-conf.h copy committed under a filename that is visually
identical to echosrv-conf.h but referenced by nothing.
num_connections was incremented before fork(), but the fork-failure
path only logged and closed the socket - no child exists to decrement
the counter, so each failed fork permanently inflates it. Under fork
failure conditions (process limits, memory pressure) the listener
eventually reports max_connections reached and refuses all connections
until restart.
tcp_listener() accepts on endpoint[active_endpoint] but then used
endpoint-> - the base of the array, i.e. endpoint[0] - for the
connection counter, the max_connections limit, SIGCHLD accounting and
start_shoveler(). With several listen addresses, every listener
process therefore:
- enforces listener 0's max_connections (verified: a listener with
limit 100 drops its second connection when listener 0 has limit 1),
- and passes listener 0's endpoint_cfg to children, so e.g. the
proxyprotocol flag of listener 0 decides whether a PROXY header is
expected on sockets accepted by other listeners.
Use a pointer to the active endpoint throughout the loop.
Sockets passed by systemd socket activation never get endpoint_cfg set,
but inc_listen_connections() dereferences it unconditionally at accept()
time. The first connection to a socket-activated unit therefore crashed
the daemon with a NULL pointer dereference, and probe_client_protocol()
dereferences it as well for the proxyprotocol check.
Verified with a stub sd_listen_fds(): sslh-select segfaults (SIGSEGV) on
the first connection to an activated socket before the fix, and forwards
the connection normally after it.
Assign a static zeroed sslhcfg_listen_item so these sockets behave as if
configured with no max_connections limit and no proxyprotocol.
cfg->host is static configuration, so an over-long path is a config
error: validate it once at startup in config_sanity_check() for both
is_unix listen endpoints and is_unix protocol targets, instead of at
bind()/connect() time. This also fails fast for protocol targets,
where the previous runtime check only dropped each connection instead
of reporting the bad config.
The memset()/bounded strncpy() calls stay in place: they are what make
sun_path termination structurally safe regardless of the input length.
Both start_listen_unix() and connect_unix() copied the configured
path into sun_path without checking its length and without zeroing
the sockaddr first. A path of 107+ bytes fills sun_path completely,
leaving it unterminated; bind()/connect() then read past the array
into uninitialised stack bytes (verified: garbage bytes visible in
the sockaddr passed to bind).
Fail listen startup with a clear message when the path cannot fit -
before unlink()ing whatever file currently sits at that path - and
make connect_unix() return -1 so the connection is dropped cleanly.
Zero both sockaddr structures, check the socket() return value in
connect_unix(), label unix listen endpoints AF_UNIX instead of
AF_INET, and route the "binding" message through print_message() so
it respects verbosity flags like everything else.
- is_msrdp_protocol() stored the 16-bit TPKT length in a char, so a
packet claiming length 263 (0x0107) but carrying only 7 bytes
compared 7 == 7 and matched - any sufficiently short packet whose
declared length low byte equals its size is accepted as RDP.
Keep the declared length in a uint16_t. Verified with a harness:
{0x03 0x00 0x01 0x07 ...} no longer matches.
- is_teamspeak_protocol() used strncmp(p, "TS3INIT1", len): for the
first datagram of a real TS3 client (len > 8) this compares the
string literal's terminating NUL against packet byte 8, which is a
version field, so it could never match real traffic (and reads
past the literal for len > 9). Use a fixed 8-byte memcmp prefix.
- is_openvpn_protocol() and is_wireguard_protocol() dereferenced the
packet buffer through uint16_t*/uint32_t* casts; the buffer is not
aligned, which is UB and traps on strict-alignment targets (ARM).
Use memcpy() instead, as already done elsewhere in this file.
Stefan Meinecke