* join/begin mrp protocol for attributes of mvrp and msrp within stream_activate.
* Creation of the attribute done on stream creation during es_buidler
Memory Safety: High
The netjack2_recv_opus() and netjack2_recv_int() functions had two
issues:
1. Missing recv length validation: after recv(), neither function
checked that the received data was at least sizeof(header) bytes.
A short packet would cause the pointer to advance past received
data, reading uninitialized VLA memory into the encoded buffer.
2. Integer overflow in bounds check: the expression
(active_ports-1)*max_encoded + sub_cycle*sub_period_bytes + data_size
uses sub_cycle from the network packet header. A large sub_cycle
value can overflow the uint32_t multiplication, wrapping around to
a small value and bypassing the encoded_size bounds check, leading
to an out-of-bounds write into encoded_data.
Additionally, validate that the received data is large enough for the
active_ports * data_size memcpy to prevent reading past the buffer.
Fix by adding recv length checks, using spa_overflow_mul/add for the
bounds arithmetic, and validating recv'd data covers the copy region.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Input Validation: High
The on_socket_data() handler only checked that the received packet was
at least avb_packet_header size before casting to avb_packet_iec61883,
which is larger. A packet between these two sizes would cause
out-of-bounds reads when accessing iec61883 fields like data_len.
Additionally, handle_iec61883_packet() used the data_len field from the
packet to determine how many bytes to copy into the ring buffer without
checking that the claimed data_len didn't exceed the actual received
data. A crafted packet with an inflated data_len could cause an
out-of-bounds read from the receive buffer.
Fix by requiring the minimum packet size to cover both the ethernet
header and the iec61883 header, and by validating that the claimed
payload size doesn't exceed the received data length.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Input Validation: High
The avb_mrp_parse_packet() function, used by both MSRP and MVRP
protocol handlers, had several missing bounds checks:
1. No minimum length validation: the parser began accessing packet
data at sizeof(avb_packet_mrp) without checking len was large
enough, causing out-of-bounds reads on truncated packets.
2. Unsafe loop terminator checks: the while loops checked m[0] and
m[1] without ensuring at least 2 bytes remained in the buffer.
3. Missing hdr_size bounds check: the header size returned by the
check_header callback was used to advance the pointer without
verifying it stayed within the packet bounds.
Fix by adding a minimum packet length check, using structure-size-aware
bounds checks in loop conditions, and validating hdr_size against
remaining packet data before advancing the pointer.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Input Validation: High
The maap_message() handler cast the incoming network data directly to
avb_packet_maap without checking that the received data was at least
sizeof(avb_packet_maap) bytes. The caller only validates the packet is
at least avb_packet_header size, which is smaller. A truncated MAAP
packet could cause out-of-bounds reads when accessing request_start,
request_count, conflict_start, and conflict_count fields in the probe
and defend handlers.
Fix by adding a minimum packet length check at the beginning of
maap_message().
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Input Validation: High
The adp_message() handler accessed avb_ethernet_header and
avb_packet_adp fields from network packet data without checking that
the packet was large enough to contain these structures. A truncated
ADP packet could cause out-of-bounds reads when accessing entity_id,
message_type, and other header fields.
Fix by adding a minimum packet length check before any field access.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Input Validation: High
The acmp_message() handler accessed fields of avb_ethernet_header and
avb_packet_acmp from network packet data without first checking that
the received packet was large enough to contain these structures.
A short packet could cause out-of-bounds reads when accessing packet
header fields.
The VLA-based reply buffers in reply_not_supported(),
handle_connect_tx_command(), and handle_disconnect_tx_command() also
lacked an upper bound on the packet length, allowing a packet claiming
a very large size to cause excessive stack allocation.
Fix by adding minimum length (sizeof(header) + sizeof(acmp)) and
maximum length (MTU) validation at the entry point before any field
access or buffer allocation.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Input Validation: High
The Apple MIDI command parser in module-rtp-session had two issues:
1. Insufficient minimum length check: the caller validated len >= 12,
but struct rtp_apple_midi is 16 bytes (cmd + protocol + initiator +
ssrc). Accessing hdr->ssrc on a 12-byte packet reads 4 bytes past
the received data.
2. Missing null-termination check: the name field (flexible array
member) from the network packet was passed directly to pw_log_info
with %s format and to find_session_by_addr_name for string
comparison, without verifying it contains a null terminator within
the received data. This could read past the receive buffer into
uninitialized stack memory, potentially leaking data into logs.
Fix by adding a sizeof check in parse_apple_midi_cmd and by validating
that the name is null-terminated within the received data in
parse_apple_midi_cmd_in.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Input Validation: Low
The log messages for short timing and control packets used sizeof(bytes)
(size of the ssize_t variable, always 8 on 64-bit) instead of
sizeof(packet) (the actual expected packet size). This caused misleading
log output that could mask packet truncation attacks or debugging issues
with RAOP timing/control packet validation.
Fix by using sizeof(packet) to correctly report the expected packet size.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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>
Cryptography: Low
When getrandom() fails, pw_random() silently falls back to rand() or
random_r() seeded from the system clock. This fallback produces
predictable output that should not be used for security-sensitive
operations like WebSocket key generation or network protocol IDs.
Add a warning log message when the fallback is triggered so that
administrators are aware of the degraded random number generation.
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>
