Memory Safety: High
When netjack2_recv_data() receives a packet that doesn't match the
expected data_stream or id, it logs "not our packet" and continues the
loop. However, since the previous recv() used MSG_PEEK, the packet is
not consumed from the socket buffer. This causes the loop to spin
indefinitely on the same mismatched packet, consuming 100% CPU.
A remote attacker on the same network can trigger this by sending a
single crafted NetJack2 packet with a mismatched stream or id field,
causing a denial of service on the audio processing thread.
Fix by consuming (discarding) the mismatched packet with a non-peeking
recv() before continuing the loop.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: High
Multiple AVB AECP AEM command handler functions copied network packet
data into stack buffers via memcpy(buf, m, len) without validating
that len fits within the destination buffer. A crafted AVB packet with
an oversized length could overflow the stack buffer.
Added bounds validation before each memcpy in:
- cmd-available.c: handle_cmd_entity_available_milan_v12
- cmd-get-set-configuration.c: set and get configuration handlers
- cmd-get-set-sampling-rate.c: unsolicited, invalid response, and get handlers
- cmd-get-set-stream-format.c: get and set stream format handlers
- cmd-lock-entity.c: handle_cmd_lock_entity_milan_v12
This matches the bounds checking pattern already used in
cmd-get-set-control.c, cmd-get-set-clock-source.c, and
cmd-get-set-name.c.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: High
The recv_midi function calculated MIDI buffer usage from network packet
fields (event_count, write_pos) using plain arithmetic that could
overflow on 32-bit platforms. A crafted NetJack2 packet with a large
event_count could wrap the size_t multiplication, bypassing the bounds
check and causing out-of-bounds memory access. Replaced with
spa_overflow_mul/spa_overflow_add to detect overflow before use.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: High
The stream_control_info() callback copied control->n_values floats
into stream->volume.values without checking bounds. The source allows
up to MAX_VALUES (256) entries but the destination volume array is
only CHANNELS_MAX (64) entries, so a stream with more than 64 channel
volumes would overflow the buffer. Clamp n_values to CHANNELS_MAX
before the copy.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: High
The handle_get_avb_info_common() function copied network packet data
into a stack buffer using memcpy(buf, m, len) without validating that
len fits within the 2048-byte buffer. A crafted AVB packet with a
large length could overflow the stack buffer. Added bounds validation
matching the pattern already used in handle_read_descriptor_common().
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: High
The cmd_names[] array was indexed with a network-provided command type
value before the bounds check, allowing an out-of-bounds read when
processing crafted AVB network packets. Moved the bounds validation
before the array access to prevent reading past the end of the array.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: High
The VBAN MIDI parse_varlen() function performed unbounded left-shifting
of a uint32_t value without overflow checking, allowing a crafted VBAN
network packet to cause integer overflow. This could produce incorrect
size calculations in get_midi_size(), leading to out-of-bounds memory
access when processing MIDI packets.
Added overflow guard (value > UINT32_MAX >> 7) matching the existing
fix in the RTP MIDI implementation, plus an overflow check on the size
addition in get_midi_size() and an avail bounds check.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Information Disclosure: Low
The RAOP module's connection cleanup frees the Digest authentication
nonce and realm strings without clearing them first. The nonce is
cryptographic material used in the Digest auth response, and the realm
is combined with the password to produce the h1 hash. After free(),
this data persists in heap memory and could be recovered through memory
disclosure vulnerabilities or core dumps.
Apply explicit_bzero before freeing, consistent with the existing
treatment of impl->password in the module destroy path.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: High
The NetJack2 driver and manager modules use VLA (variable-length array)
stack buffers sized by peer->params.mtu in every send and receive
function. In the driver module, this MTU value comes directly from the
remote peer via nj2_session_params_ntoh() without any upper bound
validation. A malicious remote peer could advertise an extremely large
MTU value (up to UINT32_MAX), causing multi-gigabyte VLA stack
allocations that overflow the stack.
Both modules also read net.mtu from user properties via
pw_properties_get_uint32() without capping the value, even though
MAX_MTU (9000) was already defined but never enforced.
Add MTU validation against MAX_MTU in the driver's session setup
handler, and cap the configured MTU value in both driver and manager
initialization.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Information Disclosure: Medium
The RAOP authentication header construction leaves sensitive material
on the stack after the function returns: Base64-encoded credentials in
enc[], MD5 password-derived hashes in h1/h2/resp[], and the assembled
Authorization header in auth[]. These persist in stack memory and can
be recovered via core dumps, memory disclosure vulnerabilities, or
cold boot attacks.
The plaintext password buffer (buf[]) was already properly cleared with
explicit_bzero, but the derived credential buffers were not. Apply
explicit_bzero to enc, h1, h2, resp, and auth before returning.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Information Disclosure: High
The RTSP client logs all HTTP headers and full RTSP request messages
at INFO level, which includes Authorization headers containing
credentials (Base64-encoded for Basic auth, hash responses for Digest
auth). The WWW-Authenticate challenge header with realm and nonce
values is also logged at INFO level.
INFO-level logs are commonly collected by system logging daemons and
may be stored in world-readable log files, exposing credentials.
Downgrade all three logging calls to DEBUG level, which is only
enabled during explicit debugging sessions.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Make a function like alloca but with overflow checks and a max
allocation size.
Use this function where we can and also make sure that all alloca calls
are in some way limited.
Memory Safety: Medium
The strlen() return value (size_t) is stored in an int before being
passed to alloca(). If a malicious client sets an extremely long
PW_KEY_NODE_NAME property, the addition could overflow the int,
resulting in a small or negative alloca size and a subsequent buffer
overflow in snprintf().
Change the type to size_t and add a bounds check to prevent
excessively large stack allocations.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: High
The add_stream_group() function computes a buffer size from the sum of
multiple string lengths, including user-controlled dictionary values
(media role, app name, etc.), and passes it to alloca() without any
bounds check. A malicious client could send very long property strings
causing an integer overflow in the size computation (wrapping a
negative/small int) or an excessively large stack allocation, leading
to a stack overflow.
Add a bounds check to reject sizes that are negative or exceed 1024
bytes before calling alloca().
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
File and Resource Handling: Medium
The PulseAudio protocol server socket initialization uses stat() to
check for existing socket files before unlinking and rebinding. This has
the same two vulnerabilities recently fixed in module-protocol-native:
1. stat() follows symlinks, so an attacker who places a symlink at the
socket path (e.g., in XDG_RUNTIME_DIR/pulse/) can trick the daemon
into unlinking the symlink target.
2. The gap between stat() and unlink() creates a TOCTOU race window.
Fix by using lstat() instead of stat() and adding an explicit symlink
check that refuses to proceed if the path is a symbolic link, consistent
with the approach in module-protocol-native.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: Medium
Two alloca() calls in the PulseAudio protocol server were missed by the
previous alloca bounds-checking fix (commit 0d2877c0d):
1. fill_node_info_proplist() adds n_items counts from node properties
and client properties without checking the total before alloca().
A client with a very large number of properties can exhaust the stack.
2. fill_card_info() uses pi->n_props from port info for an alloca()
without bounds checking. A card object with many port properties can
similarly exhaust the stack.
Add MAX_ALLOCA_SIZE checks consistent with the existing pattern to
prevent stack overflow from large property counts.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
File and Resource Handling: Medium
The lock_socket() function opens the lock file without O_NOFOLLOW. If an
attacker places a symlink at the lock file path, open() follows it and
creates or truncates a file at the symlink target with the caller's
privileges. While the lock path is typically in a user-owned runtime
directory, adding O_NOFOLLOW is a low-cost defense-in-depth measure that
prevents symlink attacks in case the directory permissions are
misconfigured or the path is influenced by untrusted input.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
File and Resource Handling: Medium
The socket initialization code uses stat() followed by unlink() to clean
up stale socket files. This creates two issues:
1. stat() follows symlinks, so an attacker who places a symlink at the
socket path can trick the daemon into unlinking an arbitrary file.
2. The gap between stat() and unlink() creates a TOCTOU race window
where the file could be replaced between the check and the removal.
Fix by using lstat() to detect symlinks without following them, refusing
to proceed if the path is a symlink, and only unlinking files that are
actually sockets (S_ISSOCK). This narrows the race window and prevents
symlink-based file deletion attacks.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: High
In get_midi_size(), the overflow check for SysEx and meta-event message
sizes has the comparison operator inverted. The check
(unsigned int)(INT_MAX - size - 1) > value
rejects small (safe) payload sizes and accepts large sizes that cause
signed integer overflow in the subsequent size += (int)value + 1.
This means all SysEx messages (0xF0, 0xF7) and system reset/meta events
(0xFF) with valid payloads are incorrectly rejected, while crafted
packets with very large variable-length values bypass the check. Although
the caller has a secondary bounds check that mitigates most exploitation,
the inverted check is both a functional bug (breaks SysEx over RTP) and
a defense-in-depth failure.
Fix by swapping the operands so that the check correctly rejects values
that would overflow: value > (unsigned int)(INT_MAX - size - 1).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: High
The nj2_dump_session_params() function logs char array fields (type,
name, driver_name, follower_name) from network-received
nj2_session_params structs using %s format. These fields are fixed-size
char arrays filled by recvfrom() and are not guaranteed to contain a null
terminator. A malicious peer can send a packet with no null bytes in
these fields, causing pw_log_info to read past the struct boundary,
potentially crashing the process or leaking adjacent heap memory.
Use %.*s format specifier with explicit maximum lengths in the dump
function to bound the string reads. Also force null-terminate the
string fields in nj2_session_params_ntoh() so that all downstream
consumers after byte-order conversion are safe.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: Critical
In handle_iec61883_packet(), the data_len field from an incoming network
packet is converted via ntohs() and then unconditionally has 8 subtracted
from it. If an attacker sends a malformed AVB packet with data_len < 8,
the subtraction wraps the uint32_t n_bytes to a very large value
(~4 billion). This corrupted size is then passed to
spa_ringbuffer_write_data(), which can overwrite the ring buffer and
adjacent heap memory with attacker-controlled network data.
Add a bounds check to verify data_len >= 8 before the subtraction,
returning early on malformed packets.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: Medium
Several functions in the PulseAudio protocol implementation use alloca()
to allocate arrays of port_info, profile_info, or dict_item structs
based on counts derived from card parameters or client property lists.
These counts have no upper bounds, so a card object with a very large
number of parameters or a client sending many properties can cause
alloca() to exhaust the stack, resulting in a stack overflow crash.
Add a MAX_ALLOCA_SIZE (64KB) limit and check element counts before each
alloca() call. If the requested allocation exceeds the limit, the
function returns -ENOMEM instead of crashing.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
File and Resource Handling: High
The pipe-tunnel module creates FIFOs and then adjusts their
permissions using chmod() on the path. Between mkfifo() and
chmod(), an attacker with write access to the directory (e.g.,
/tmp with the default hardcoded paths) can delete the FIFO and
replace it with a symlink to a target file. The chmod(0666) then
changes permissions on the symlink target, allowing the attacker
to make arbitrary files world-readable/writable.
Fix by:
1. Adding O_NOFOLLOW to the open() call so symlinks are rejected
at open time rather than followed.
2. Moving the permission change from chmod() (path-based, follows
symlinks) to fchmod() (fd-based, operates on the already-opened
and validated file descriptor), and doing it after the fstat
S_ISFIFO check confirms we opened an actual FIFO.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: Medium
In both module-netjack2-driver.c and module-netjack2-manager.c, the
socket buffer size is computed as:
NETWORK_MAX_LATENCY * (mtu + period_size * sizeof(float) * n_ports)
This arithmetic is performed in int (signed 32-bit) but the
intermediate values can exceed INT_MAX with large but valid network
parameters. Signed integer overflow is undefined behavior in C,
and the resulting negative value passed to setsockopt would set an
incorrect socket buffer size.
Fix by widening the intermediate computation to size_t and clamping
the result to INT_MAX before storing in the int variable.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: Medium
In update_delay(), the delay compensation size is computed as
delay * sizeof(float) where delay is int64_t but size is uint32_t.
When the delay value is very large, the multiplication result
truncates to a small uint32_t value. This causes an undersized
buffer allocation in resize_delay(), while compensate_samples
retains the original large value. Subsequent use of
compensate_samples could then write past the end of the buffer.
A negative delay (possible if delay_samples overflows) would also
produce a large unsigned size due to implicit conversion.
Fix by clamping the delay to be non-negative and within the maximum
delay buffer size before the multiplication, ensuring the size
cannot truncate or wrap.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: Medium
In recalculate_buffer(), the buffer size computation
(delay + (1u<<15)) * 4 can overflow uint32_t when delay is large.
Additionally, the subsequent multiplication buffer_size * channels
for the realloc size can also overflow uint32_t, wrapping to a
small value and causing an undersized allocation. Later writes to
the buffer using the logical buffer_size would then overflow the
heap.
The delay value derives from rate * target_delay where both values
come from stream negotiation properties.
Fix by adding an overflow check on the delay computation and
widening the allocation size to size_t to prevent the second
overflow.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: High
In netjack2_init(), several buffer sizes are computed by multiplying
network-provided session parameters (period_size, channel counts,
kbps) without overflow checks. A malicious network peer can send
crafted session parameters that cause these multiplications to
overflow, resulting in undersized buffer allocations. Subsequent
writes to these buffers then overflow the heap.
Specific issues fixed:
1. midi_size = period_size * sizeof(float) * max_midi_channels can
overflow, causing calloc to allocate a small buffer.
2. encoded_size = max_encoded_size * max_audio_channels can overflow
for both INT and OPUS encoders.
3. OPUS kbps * period_size * 1024 numerator can overflow uint32_t;
widen to uint64_t for the intermediate calculation.
4. Division by zero if sample_rate is 0 in OPUS encoder path.
5. Missing NULL checks on calloc for empty and midi_data buffers.
6. Channel counts not capped to MAX_CHANNELS before use.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: High
In vban_midi_receive(), the received buffer is cast to struct
vban_header and its n_frames field is accessed before validating
that the packet is large enough to contain the header. A truncated
packet shorter than VBAN_HEADER_SIZE would cause an out-of-bounds
read.
Fix by checking that len >= VBAN_HEADER_SIZE before accessing the
header, matching the fix applied to vban_audio_receive().
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: High
In vban_audio_receive(), the received buffer is cast to struct
vban_header and its fields are accessed before validating that the
packet is large enough to contain the header. If a truncated packet
shorter than VBAN_HEADER_SIZE is received, this reads past the end
of the buffer.
Additionally, when len < hlen, the plen calculation (len - hlen)
produces a negative ssize_t value which, when used in the unsigned
division plen / stride, gets implicitly converted to a very large
value, potentially causing further out-of-bounds reads.
Fix by checking that len >= VBAN_HEADER_SIZE before accessing the
header.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Information Disclosure: Medium
The MD5_hash() function formats password material into a 1024-byte
stack buffer for hashing but never clears it afterward. Similarly,
the Basic auth path in rtsp_add_raop_auth_header() formats
username:password into a stack buffer without clearing it.
These buffers remain on the stack after the functions return, and
could be exposed through memory disclosure vulnerabilities, core
dumps, or memory inspection.
Clear the buffers with explicit_bzero() immediately after they are
no longer needed, consistent with the existing practice of clearing
the password before freeing in impl_destroy().
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: High
In netjack2_recv_float(), several values from untrusted network packet
headers are used in arithmetic without overflow protection:
1. active_ports from the network header had no upper bound check. A
very large value causes `active_ports * sub_period_bytes` to
overflow uint32_t, producing a small value that passes the length
check, then the loop iterates out of bounds on the receive buffer.
2. The sub_cycle bounds check `sub_cycle * sub_period_size >
quantum_limit` can overflow, allowing a large sub_cycle to pass
the check and cause an out-of-bounds write when computing the
destination offset.
Fix by capping active_ports to MAX_CHANNELS, casting to size_t for the
length check to prevent overflow, and rewriting the sub_cycle check as
a division to avoid overflow.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: High
In netjack2_recv_midi(), the offset calculation `max_size * sub_cycle`
uses sub_cycle from an untrusted network packet header. A large
sub_cycle value could cause integer overflow, producing a small offset
that passes the subsequent bounds check and leads to an out-of-bounds
write into the MIDI data buffer.
Similarly, the bounds check `offset + len < midi_size` could itself
overflow, and the `used` size calculation from network-controlled
event_count and write_pos fields could overflow to bypass the size
check.
Fix by adding an explicit overflow check before the multiplication,
rewriting the bounds check to use subtraction (which cannot overflow
after the prior check), and adding an underflow check on the `used`
calculation.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: High
In ensure_size(), the check `m->length + size <= m->allocated` could
overflow when both m->length and size are large uint32_t values,
wrapping around to a small number and incorrectly passing the bounds
check. This could allow writing past the end of the allocated buffer.
Rewrite the check as `size <= m->allocated - m->length` which cannot
overflow since we already verified m->length <= m->allocated. Also add
an explicit overflow check for the new allocation size calculation.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Information Disclosure: Medium
The RAOP authentication password was freed without first clearing the
memory contents. This leaves the plaintext password in freed heap
memory where it could be recovered by an attacker with access to
process memory (e.g. via /proc/pid/mem, core dumps, or a separate
memory safety vulnerability).
Use explicit_bzero() to securely clear the password before freeing.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: High
In rtsp_do_options_auth(), the return values of strdup() for
auth_method, realm, and nonce were not checked for NULL. If strdup()
fails due to memory exhaustion, spa_streq() on auth_method will
dereference NULL, and the realm/nonce pointers will be used later in
MD5_hash() causing NULL pointer dereferences.
Add NULL checks after each strdup() call, returning -ENOMEM on failure.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: High
Three malloc calls for ring buffers (rec_buffer, play_buffer,
out_buffer) had no NULL checks. If any allocation fails, the
NULL pointers would be passed to memset and ringbuffer
operations in reset_buffers(), causing a NULL pointer
dereference crash.
Additionally, the ring size calculations used uint32_t
arithmetic which could overflow with large user-configurable
buffer.max_size values. Cast to size_t to perform the
multiplication in 64-bit, preventing intermediate overflow.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
