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>
Input Validation / Memory Safety: Medium
The RTSP client used for RAOP/AirPlay communication accepted arbitrarily
large Content-Length values from the remote server without any upper
bound. A malicious or compromised AirPlay server could specify a very
large Content-Length, causing the client to allocate unbounded memory
and potentially exhaust system resources (denial of service).
Additionally, the return value of pw_array_add() was not checked. If
the allocation failed, the subsequent memcpy would dereference a NULL
pointer, causing a crash.
Add a 64KB limit on Content-Length (more than sufficient for RTSP
control messages) and check the pw_array_add return value.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
rtp_stream_new() acquires a data loop with pw_context_acquire_loop() but
the out: error path never calls pw_context_release_loop(), leaking the loop
reference on every failure after acquisition.
Mirror rtp_stream_destroy() and other modules that pair acquire with release.
When adding the freeze/thaw recalc graph calls, triggering the actual
recalc of the graph (when an active node with a driver was removed) was
removed.
The thaw only actually recalcs the graph when something set the
recalc_pending flag so we still need something to set this. Normally
this would be because of some of the ports or links that got destroyed
but if not, the original recalc trigger needs to remain as a fallback.
Some operations like deactivating nodes deactivates all links to the
node and this causes a graph recalc for each link.
It's actually a bit more problematic with suspend, which first
deactivates the links and then suspends the ports. The suspension of the
ports cause a recalc, which for the not yet suspended ports makes the
link active again, causing issues then when the port is suspended later.
Make recalc a counter and only recalc when the counter is 0. If the
counter is not 0, set the pending recalc, which will trigger when the
counter goes to 0.
With this, we can add a freeze/thaw operation on the recalc and delay
recalculation until all ports and links are handled.
We can also group some other operations with a freeze/thaw pair, such as
the destruction of ports, which destroys all links. Also the destruction
of nodes can freeze the recalc until all ports are destroyed.
update_delay is called primarily when the stream format or latency changes,
and from playback thread, if stream reports different delay as before.
This function calculates the number of compensate samples for each stream
based on the latencies of other streams (which must be in a streaming state).
During the first playback on a new format, update_delay is called multiple times
due to format or latency changes. The delay is calculated only from streams
that are currently streaming. If some streams are not yet streaming, their
latencies are ignored, and the delay is updated later in the processing
thread. The processing thread also stores the stream delay in a local variable
(accessed only from that thread, thus requiring no locking).
On a subsequent playback using the same format, update_delay is still called a
few times, and the delay is updated based on the currently streaming streams.
If some streams are not streaming, their latencies are ignored.
However, this time, the processing thread fails to update the delay for the
previously non-streaming streams. Because the format didn't change, the streams
delay matches the last stored delay from the previous playback. As a
result, the compensate samples are not recalculated.
To properly update the compensate samples, update_delay must also be called
when a stream's state changes to streaming (avoiding the need to clear the
thread-buffered value, which would require locking in the processing
thread).
In multiple cases the `flags` member of `spa_dict` is left unitialized,
so try to avoid that. For example in `fill_node_info_proplist()` it is
accessed in `spa_dict_lookup_item()`.
This also modifies `collect_props()` to not depend on a partially initialized
`dict` parameter.
Bring the port to INIT before going to CONFIGURE when we do the suspend
logic.
This is to ensure that there always is a state change to emit the state
change notification. This was, the link can detect the suspend case and
cancel any pending results.
One of the problem with suspend is that the state changes on the ports
are done from different places; the port and link. This causes issues
like:
1. do_negotiate calls pw_port_set_param(Format,..) with the negotiated
format. This returns async and the link queues a complete_ready
callback.
2. The node is suspended, pw_impl_port_set_param(Format, NULL) is called
to clear the port format. This bypasses the link.
3. The reply from step 2 arrives and triggers complete_ready, this
brings the port state to READY and the link state to ALLOCATING.
4. The link continues allocating and sets buffers on the port. This then
fails because the last format set was NULL.
Ideally all port states should be managed in one place and the async
port state changes should be kept in the port itself as well but this
will need some more work.
Fixes#3547
Thse should not be portal clients and can go directly to rtkit.
The reason is that there is a locking problem in the portal rt
implementation that cause timeouts for some reason.
When one of the nodes from a group is running, all the other nodes in the
group should run as well.
The link group is only for the session manager to avoid linking the same
link group to avoid loops, like linking the output of a loopback to the
input.
The problem is that we removed the node.link-group from driver nodes (in
ACP and elsewhere) but now the source/sink pairs (when in the same group
like for pro-audio) don't run together in all cases, causing sync issues.
See #5210
Keep the flag dont_inhibit_auto_suspend around and use it do decide when
to send suspend messages to the client.
We don't always want to send suspend messages when the stream state changes
because that could happen because the stream was, for example, relinked.
The intention of the suspend message is mostly for monitor streams that
use the dont-inhibit flag and want to follow the suspend state of the
sink.
See #5273
Instead of just setting the port state directly, go through the
port_update_state() function. This will make suspend_node() also
emit the port state change event to the links. This then causes
the link to reset to INIT, which cancels pending complete_ready work
items and prevents them from changing the port to READY without a
valid format.
One possible situation where this could happen is when a suspend
happens while the port was waiting for a set_param(Format) reply from
the client-node.
See #3547
Make a new library.filter-path for the filter-graph that will filter and
restrict the dlopen filenames (used for the LADSPA plugin only).
By default this is false and so filter-chain can load from absolute
paths without extra checks.
Enable the extra checks for the pulse LADSPA modules and the
audioconvert filter graphs because these allow loading LADSPA plugins
into other processes.
Fixes#5222
There is no limit on the number of inputs/outputs of a graph but the
filter-chain assumes it is at most 128 and also that there are at most
128 buffer datas.
Increase the limit (1024) and clamp and log an error when the
filter-graph has more channels. Also clamp the buffer datas so that we
don't overflow the stack allocated buffers.
The aec_method parameter is interpolated into a SPA library path
as "aec/libspa-aec-%s". A client could use "../" sequences to
load arbitrary SPA plugins. Reject values containing ".." or "/".
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A PulseAudio client can load this module with an arbitrarily complex
blocklist regex, causing catastrophic backtracking in regexec on
every new device. Cap the regex string at 1024 characters.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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>
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 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>
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>
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>
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>
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>
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>
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: High
The read_arbitrary() bounds check used `m->offset + len > m->length`
where len is an attacker-controlled uint32_t read from the PulseAudio
protocol message. When m->offset is small and len is close to
UINT32_MAX, the addition wraps around to a small value, bypassing
the bounds check. This allows read_arbitrary() to return a pointer
within the message buffer but report an enormous length to the caller,
leading to out-of-bounds memory reads.
Fixed by rearranging the arithmetic to use subtraction:
`len > m->length - m->offset`, which cannot overflow since
m->offset <= m->length is maintained as an invariant.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
File and Resource Handling: Medium
Several file and socket operations were missing the close-on-exec flag,
which causes file descriptors to leak to child processes created via
fork+exec. This could allow child processes unintended access to
privileged resources.
- node-driver.c: SOCK_DGRAM socket for SIOCETHTOOL ioctl leaked to
child processes
- pw-container.c: Unix domain listen socket leaked to spawned
container processes
- compress-offload-api.c: ALSA compress-offload device fd leaked to
child processes
Added O_CLOEXEC to open() calls and SOCK_CLOEXEC to socket() calls.
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
When the system does not provide reallocarray(), pw_reallocarray()
falls back to realloc(ptr, nmemb * size). The multiplication
nmemb * size can silently overflow, causing a smaller-than-expected
allocation. Subsequent writes to the allocation then overflow the
heap buffer.
This function is used extensively throughout PipeWire for allocating
arrays from protocol data, making it a wide attack surface.
Fix by adding an explicit overflow check before the multiplication
in the fallback path, matching the behavior of the real
reallocarray().
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
