For playback and capture streams we allocate MAXLENGTH (4M) buffers but
for upload streams we must allow space for the total upload stream, which
can be up to the max allowed sample size (16M).
Keep the allocated size for the stream around in a variable so that we
can use it when writing/reading to/from the ringbuffer.
This could later also be extended to use the attr.maxlength variable to
size the buffer (but it's usually 4M anyway). This is more complicated
because we need to grow the buffer size when new attributes are set,
which is probably more complicated than useful.
Use the JSON builder to prepare arguments for modules and metadata
instead of custom memopen and fprintf. This makes it easier to ensure
the strings are all properly escaped.
This removes the use of spa_json_encode_string(), which could return a
truncated, non-zero terminated result, which we needed to check
everywhere.
spa_json_encode_string was called with sizeof(name)-1, which would
not write a null terminator on truncation. Use sizeof(name) and skip
sink names that don't fit in the buffer.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
pw_properties_new can return NULL under OOM. The result was used
directly without a check, leading to a NULL pointer dereference.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
In the SPA_CHOICE_Enum case, values[index+1] was used to skip the
default value at index 0, but the bounds check only validated index,
not index+1. Move bounds checks into each case with the correct limit.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The listen address was inserted into a JSON array without escaping.
Build the address string first, then encode it with
spa_json_encode_string.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The switch in message_get had no default case. An unrecognized tag byte
from a malicious client would skip the switch body without consuming
the va_arg parameter, desynchronizing all subsequent argument reads
and causing undefined behavior.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
do_cork_stream, do_flush_trigger_prebuf_stream, and do_set_stream_name
did not check whether the stream had completed format negotiation.
Add create_tag guards matching the pattern in do_set_stream_buffer_attr.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A trailing backslash in a module argument string would cause the
escape handling to advance past the null terminator, reading one
byte out of bounds on the next loop iteration.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
strndup can return NULL under OOM. The result was passed directly to
spa_json_begin_array which would dereference the NULL pointer.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
If a client sends UPDATE_PLAYBACK_STREAM_SAMPLE_RATE before format
negotiation completes, stream->ss.rate could be 0, causing a
floating-point division by zero. Add the same create_tag guard used
in do_set_stream_buffer_attr.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Client-supplied int64_t offset was multiplied by 1000 without overflow
check. Use spa_overflow_mul to detect and reject values that would
overflow.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The create_tag guard added in a2de6c886 also rejected memblocks for
upload streams, which never clear create_tag. Upload streams allocate
their buffer immediately, so the NULL deref risk does not apply to
them. Exempt STREAM_TYPE_UPLOAD from the check.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
operation_complete removed the operation from the list and freed it
but never decremented client->n_operations. After 64 completed
operations the client would be permanently locked out.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Use spa_json_encode_string to escape user-supplied strings before
inserting them into JSON configs in module-always-sink,
module-x11-bell, and module-switch-on-connect.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Both reply_new and reply_error passed the message_alloc result directly
to message_put without checking for NULL, which would cause a NULL
pointer dereference on allocation failure.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The listen address was inserted into JSON without escaping. Build the
address string first, then encode it with spa_json_encode_string to
prevent injection of arbitrary JSON keys.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The plugin and label parameters in module-ladspa-sink and
module-ladspa-source were inserted into the filter-chain JSON config
without escaping. Use spa_json_encode_string to prevent injection.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
read_cvolume accepted channels=0, creating a degenerate zero-length
volume array that is passed to pw_stream_set_control and SPA pod
building. Reject zero channels alongside the existing CHANNELS_MAX
upper bound check.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was no limit on concurrent PLAY_SAMPLE operations per client.
Each creates a PipeWire stream, allowing a client to exhaust server
resources. Add a MAX_PENDING_SAMPLES (64) limit per client.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was no limit on pending operations per client. Commands like
SET_SINK_VOLUME each allocate an operation that persists until a
manager sync completes. A client flooding these commands can exhaust
server memory. Add a MAX_OPERATIONS (64) limit per client.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
If module_create succeeded but the subsequent calloc for
pending_module failed, the module was leaked in the modules map.
Move the calloc before module_create so failure cleanup is trivial.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The device name was interpolated into a JSON metadata string without
escaping. A node with crafted name containing quote characters could
inject arbitrary JSON keys into the default sink/source metadata.
Use spa_json_encode_string to properly escape the value.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A client can create a stream with invalid sample_spec (rate=0) via
format_info negotiation, then send SET_STREAM_BUFFER_ATTR before
negotiation completes. fix_playback_buffer_attr divides by ss.rate,
crashing the daemon. Reject buffer attr changes on streams that
have not completed format negotiation.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The client-provided rate was used without validation. A zero or
excessively large rate produces extreme correction values passed
to pw_stream_set_control. Reject rates that are zero or exceed
RATE_MAX.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
pw_properties_new can return NULL on OOM. Passing NULL to read_props
causes a NULL pointer dereference through pw_properties_set. Only
reachable when debug logging is enabled.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
message_alloc can return NULL on allocation failure but the result
was not checked, causing the next do_read call to misinterpret
the NULL as a protocol error instead of an OOM condition.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was no limit on the total size of the sample cache. A client
could upload many samples to exhaust server memory. Add a configurable
pulse.max-sample-cache property (default 64MB) to cap the total size
of all cached samples.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was no limit on the number of streams a single client could
create. Each stream allocates a 4MB ring buffer, allowing a malicious
client to exhaust server memory. Add a configurable pulse.max-streams
property (default 64) to limit streams per client.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The device_name from a client message was interpolated directly into
a JSON string without escaping. A malicious client could inject
arbitrary JSON keys by including quote characters in the device name.
Use spa_json_encode_string to properly escape the value.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A client can send memblock data to a playback stream channel before
format negotiation completes and the stream buffer is allocated,
causing a NULL pointer dereference crash. Reject memblock data for
streams that are still being created (create_tag != SPA_ID_INVALID).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The sample cache upload buffer is allocated as MAXLENGTH (4MB) but
sample->length can be up to SCACHE_ENTRY_SIZE_MAX (16MB). During
playback, the read offset can exceed the buffer size, causing an
out-of-bounds heap read. Wrap the offset into the ring buffer.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
format_info_to_spec parses the format.channel_map property without
checking against CHANNELS_MAX (64) before writing to map->map[].
A client supplying more than 64 channel names overflows the stack-
allocated channel_map buffer.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
File and Resource Handling: Medium
In on_connect(), if client_new() fails or pw_loop_add_io() fails, the
accepted client_fd is never closed. The error path only calls
client_free() which relies on pw_loop_destroy_source() to close the fd,
but if the source was never created, the fd leaks.
Fix by closing client_fd in the error path when it has not been
transferred to a loop source.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: Medium
Two calloc() calls in backend-native.c do not check the return value
before dereferencing the pointer:
1. rfcomm_send_cmd_enqueue() allocates an rfcomm_cmd struct and
immediately passes cmd->cmd to vsnprintf without a NULL check.
2. rfcomm_hfp_ag_clcc() allocates an updated_call struct and
immediately dereferences updated_call->id without a NULL check.
Both would crash on allocation failure. Add NULL checks that return
an error instead of dereferencing NULL.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: Low
The udev device enumeration code uses alloca(strlen(str) + 1) to
allocate stack buffers for unescaping ID_VENDOR_ENC and ID_MODEL_ENC
udev properties. These property values originate from the udev database
and could theoretically be manipulated through custom udev rules or
crafted USB device descriptors. An excessively long property value
would cause unbounded stack allocation.
Add a 1024-byte cap on the alloca size and skip the unescape step for
oversized values, falling back to the raw encoded string.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: Medium
parse_graph() does not check the return values of calloc() for
input_names/output_names arrays, or strdup() for individual name
entries. If any allocation fails, the code dereferences a NULL pointer
or stores NULL without detection. Add NULL checks that return -ENOMEM
on allocation failure.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: Medium
ladspa_plugin_make_desc() calls calloc() twice without checking the
return value. If either allocation fails, the code dereferences a NULL
pointer, causing a crash. Add NULL checks after both calloc calls and
properly free the descriptor struct if the ports allocation fails.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add MAX_RECV_PACKETS limit to both sync_wait functions to prevent
busy-spinning on the real-time thread under a packet flood, where
SO_RCVTIMEO never fires because data is always available.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Memory Safety: Medium
The parse_permissions_struct macro in protocol-native uses alloca()
to allocate space for permissions received from protocol messages.
With MAX_PERMISSIONS=4096 and sizeof(struct pw_permission)=8, this
could allocate up to 32KB on the stack from a single message. Combined
with parse_dict (up to 16KB), a crafted message could consume ~48KB
of stack space.
Reduce MAX_PERMISSIONS from 4096 to 1024 (matching MAX_DICT) to limit
the maximum stack allocation to 8KB. This is still more than sufficient
for any legitimate permission update - typical systems have far fewer
than 1024 objects that need individual permission entries.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Input Validation: High
The netjack2_recv_data loop terminates based on the is_last flag
from received network packets. A malicious peer could continuously
send packets with is_last=0, causing the receive loop to run
indefinitely and blocking the audio processing thread. This is
a denial of service vulnerability.
Add a maximum packet count (1024) per receive cycle. This is
well above what any legitimate netjack2 session would produce
but prevents a malicious peer from stalling the processing thread.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Memory Safety: Critical
write_event() returns a negative int on error (-ENOSPC or -ERANGE),
but its return value was added directly to the uint32_t len variable
without checking. A negative return value would wrap len to a very
large number due to unsigned integer conversion, causing subsequent
buffer writes to go far out of bounds. This could lead to stack
corruption and potential code execution.
Fix by checking the return value of write_event() before using it.
If write_event() fails, abort the flush operation safely.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
netjack2_driver_sync_wait can return an negative value on error, don't
do any processing in that case instead of wrapping around the negative
value into a huge unsigned int and breaking things..
