Commit graph

15180 commits

Author SHA1 Message Date
Wim Taymans
807b93fb05 security: add per-client pending sample limit in PulseAudio protocol
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>
2026-04-29 17:19:08 +02:00
Wim Taymans
138e30df38 security: add per-client operation count limit in PulseAudio protocol
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>
2026-04-29 17:17:38 +02:00
Wim Taymans
f32295429f security: fix module leak on OOM in PulseAudio do_load_module
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>
2026-04-29 17:13:56 +02:00
Wim Taymans
2d8dc8b457 security: fix JSON injection in PulseAudio do_set_default
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>
2026-04-29 17:09:50 +02:00
Wim Taymans
d3e1be8b6e security: fix division by zero in PulseAudio set_stream_buffer_attr
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>
2026-04-29 17:08:17 +02:00
Wim Taymans
cd7bb1e37d security: validate sample rate in PulseAudio update_stream_sample_rate
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>
2026-04-29 16:51:46 +02:00
Wim Taymans
5f02641859 security: add missing NULL check in PulseAudio message_dump
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>
2026-04-29 16:45:09 +02:00
Wim Taymans
9c1bc64af4 security: add missing NULL check after message_alloc in PulseAudio server
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>
2026-04-29 16:42:59 +02:00
Wim Taymans
52afec565b security: add total sample cache size limit in PulseAudio protocol
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>
2026-04-29 16:39:57 +02:00
Wim Taymans
37990b5e90 security: add per-client stream limit in PulseAudio protocol
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>
2026-04-29 16:28:05 +02:00
Wim Taymans
80ec1f1d10 security: fix JSON injection in PulseAudio stream-restore
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>
2026-04-29 16:16:44 +02:00
Wim Taymans
a2de6c886e security: fix NULL dereference in PulseAudio handle_memblock
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>
2026-04-29 16:12:49 +02:00
Wim Taymans
808bcf39cd security: fix heap OOB read in PulseAudio sample cache playback
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>
2026-04-29 16:10:44 +02:00
Wim Taymans
c1f6cde926 raop: handle strdup allocation error 2026-04-29 15:52:46 +02:00
Wim Taymans
d23ec56f0d security: fix stack buffer overflow in PulseAudio channel map parsing
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>
2026-04-29 15:49:50 +02:00
Wim Taymans
0c4a0dcf7d security: fix file descriptor leak in PulseAudio server on_connect error path
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>
2026-04-29 14:28:06 +02:00
Wim Taymans
398326f19c security: add missing NULL checks after calloc in Bluetooth backend
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>
2026-04-29 14:22:53 +02:00
Wim Taymans
d4cf1d0d6f security: bound alloca size for udev property strings
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>
2026-04-29 14:22:33 +02:00
Wim Taymans
6bcefd0d59 security: add missing NULL checks after calloc/strdup in filter-graph
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>
2026-04-29 14:20:46 +02:00
Wim Taymans
715d1736e9 security: add missing NULL checks after calloc in LADSPA plugin
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>
2026-04-29 14:19:59 +02:00
Wim Taymans
5f50055750 security: add iteration limit to netjack2 sync wait loops
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>
2026-04-29 14:18:41 +02:00
Wim Taymans
c21616d1ee security: check netjack2_init return value in driver
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-04-29 14:08:06 +02:00
Wim Taymans
0816d4a2fd security: reduce MAX_PERMISSIONS to limit alloca stack usage
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>
2026-04-29 13:58:04 +02:00
Wim Taymans
c3c11e4c76 security: add max packet limit to netjack2 recv_data loop
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>
2026-04-29 13:58:02 +02:00
Wim Taymans
110495ed9f security: fix unchecked write_event return value in RTP MIDI
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>
2026-04-29 13:57:45 +02:00
Wim Taymans
739e2d1107 netjack2: handle sync_wait errors
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..
2026-04-29 13:56:11 +02:00
Wim Taymans
3d414960c2 security: clamp netjack2 sync.frames to quantum limit
Clamp sync.frames to quantum_limit in both sync_wait functions so all
recv paths (float, int, opus, and the fallback memset in recv_data) use
a bounded frame count. A malicious remote could send a large sync.frames
causing buffer overflows in recv_int, recv_opus, and the unfilled-buffer
memset.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-04-29 13:42:49 +02:00
Charles
c5083e7f32 impl-core: gate portal clients on stolen-fd reconnect
When the XDG camera portal steals a PW fd and hands it to an
app, the app can connect before the session manager has set up
permissions, seeing no camera nodes.

Gate portal clients in core_hello by removing PW_PERM_R from
PW_ID_CORE, triggering the busy state so daemon stops reading from the
client socket until the session manager restores permissions.

A marker property (pipewire.access.portal.gated) notifies the
session manager to ungate after attaching the PermissionManager.
The property is cleared before each gate cycle to handle repeated
fd steals on the same client.

Only gate when the session manager has set the capability property
pipewire.access.portal.gate-supported on the client, so older session
managers that cannot ungate are unaffected. Check ALL clients in the
context rather than this client specifically because the portal may
create a brand new client for each camera session and the session
manager won't have processed it yet.

Fixes: https://gitlab.freedesktop.org/pipewire/wireplumber/-/work_items/941
2026-04-29 12:31:57 +01:00
Wim Taymans
593132e434 security: fix error path resource leaks in netjack2 manager
Fix handle_follower_available to properly clean up on all error paths
after the follower has been added to the list. Add missing NULL checks
for pw_properties_copy and check the netjack2_init return value.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-04-29 13:21:21 +02:00
Wim Taymans
4ac5364004 security: add max followers limit to netjack2 manager
Add a configurable netjack2.max-followers property (default 64) to
limit the number of concurrent followers, preventing resource exhaustion
from unbounded follower connections.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-04-29 13:10:46 +02:00
Wim Taymans
7dee2c158f security: fix integer overflow in netjack2 opus encoded size calculation
Cast the denominator to uint64_t to prevent sample_rate * 8 from
overflowing uint32_t, which could produce a tiny denominator and
an inflated max_encoded_size.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-04-29 12:39:37 +02:00
Wim Taymans
be4fe881e3 security: validate opus encoded length in netjack2 recv
Validate that the encoded length from the network does not exceed
the available encoded data region before passing it to the opus
decoder.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-04-29 12:37:55 +02:00
Wim Taymans
3a77f9c28a security: fix OOB read in netjack2 MIDI buffer parsing
Validate that the midi buffer metadata fits within the buffer size
before computing the offset, preventing a size_t underflow. Also
bounds-check non-inline event data pointers against the validated
buffer region.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-04-29 12:30:37 +02:00
Wim Taymans
0186bccdce netjack2: account for overhead
The period calculation now subtracts the per-port int32_t overhead
from max_size before computing how many float samples fit. This guarantees
active_ports * (period * sizeof(float) + sizeof(int32_t)) <= max_size, so
packet_size = sizeof(header) + active_ports * sub_period_bytes <= mtu.

sub_cycle is bounded by nframes / sub_period_size, matching the sender's
num_packets = nframes / sub_period_size. Also ensure sub_period_size != 0
to avoid division by 0.
2026-04-29 12:27:29 +02:00
Wim Taymans
2af9e879a7 netjack2: use peer params name and follower_name
peer->params.name and peer->params.follower_name are null-terminated
by nj2_session_params_ntoh, whereas the raw params->name from the network
packet had no such guarantee.
2026-04-29 11:59:55 +02:00
Wim Taymans
bc93a745ab security: add missing NULL check after strdup in MIDI server
Memory Safety: Medium

spa_bt_midi_server_new() did not check the return value of strdup()
when duplicating the characteristic path. On allocation failure, a
NULL chr_path would be returned as part of the server object,
leading to a NULL pointer dereference when later used. Add a NULL
check that jumps to the existing fail cleanup path.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-04-29 11:36:53 +02:00
Wim Taymans
acabcf085d security: add missing NULL checks after strdup/calloc in backend-hsphfpd
Memory Safety: Medium

Multiple allocation results in the HSP/HFP daemon backend were not
checked for NULL:

- transport_data->transport_path strdup in new_audio_connection()
- endpoint->remote_address and local_address strdup in property parsing
- t_path strdup before spa_bt_transport_create()
- endpoint calloc and endpoint->path strdup in interface enumeration
- backend->hsphfpd_service_id strdup after registration

Each could cause a NULL pointer dereference under memory pressure. Add
appropriate NULL checks with error returns matching the existing patterns
in each function (DBUS_HANDLER_RESULT_NEED_MEMORY or -ENOMEM).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-04-29 11:36:32 +02:00
Wim Taymans
c3c1216633 security: add missing NULL check after strdup in reserve
Memory Safety: Medium

rd_device_new() did not check the return value of strdup() when
duplicating application_name. On allocation failure, a NULL pointer
would be stored and later passed to D-Bus functions, causing a
crash. Add a NULL check that jumps to the existing error_free
cleanup path.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-04-29 11:35:31 +02:00
Wim Taymans
4de0f83aca security: add missing NULL checks after realloc/strdup in LV2 plugin
Memory Safety: Medium

Two issues in the LV2 filter-graph plugin:

1. uri_table_map(): realloc() result was assigned directly to
   table->data, losing the original pointer on failure (memory leak)
   and causing a NULL pointer dereference on the next access. Also
   the subsequent strdup() had no NULL check. Fixed by using a
   temporary pointer for realloc and checking strdup's return.

2. lv2_state_retrieve(): realloc() of sd->tmp was used without a
   NULL check, so a failed allocation would cause sd->tmp to become
   NULL and be immediately passed to spa_json_parse_stringn(). Fixed
   by checking the realloc result before assignment.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-04-29 11:35:08 +02:00
Wim Taymans
dcf28ff248 security: add missing NULL checks after strdup in modemmanager
Memory Safety: Medium

Four strdup() calls in the ModemManager Bluetooth integration had no
NULL checks, which could lead to NULL pointer dereferences under
memory pressure:

- mm_parse_call_properties(): call->number assignment
- mm_parse_interfaces(): this->modem.path assignment
- mm_filter_cb(): call_object->path assignment (also leaked calloc
  on failure)
- mm_register(): this->allowed_modem_device assignment

Each site now checks for NULL and handles the failure appropriately
for its context (early return, goto cleanup, or return error).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-04-29 11:34:37 +02:00
Wim Taymans
9a4e0e4c85 security: fix format string vulnerability in hook.h example code
Input Validation: Low

The documentation example code in hook.h passed the msg parameter
directly as the format string to printf() and fprintf(). If copied
by developers, this pattern creates a format string vulnerability
where specially crafted msg content with format specifiers (%x, %n,
etc.) could read/write memory. Use "%s" as the format string and
pass msg as a data argument instead.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-04-29 11:33:44 +02:00
Wim Taymans
7982f52830 security: replace sprintf with snprintf in spa_debugc_mem
Memory Safety: Medium

The spa_debugc_mem() function used unbounded sprintf() calls to format
hex dump output into a fixed 512-byte stack buffer. While the current
line-by-line output (16 bytes per line) fits within the buffer, sprintf
provides no overflow protection if the format changes or assumptions
are violated. Replace with snprintf() using sizeof(buffer) and remaining
space tracking to guarantee the buffer cannot be overflowed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-04-29 11:33:25 +02:00
Wim Taymans
106f641ff3 security: add missing NULL check after strdup in pw_strv_insert
Memory Safety: Medium

In pw_strv_insert(), the strdup(str) result at the insertion position
was not checked for failure. A NULL would be stored in the string
vector, causing NULL dereferences when callers iterate the vector.

Fix by checking the strdup() return value and cleaning up on failure.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-04-29 11:11:58 +02:00
Wim Taymans
e474303991 security: add missing NULL checks after strdup/strndup in pw_split_strv
Memory Safety: Medium

In pw_split_strv(), the return values of strndup() and strdup() were
passed directly to pw_array_add_ptr() without checking for NULL. If
memory allocation fails, NULL pointers would be stored in the string
array and later dereferenced by callers iterating the result.

The return value of pw_array_add_ptr() was also not checked, which
could lead to silently dropped strings.

Fix by checking both allocation and array insertion return values,
and properly cleaning up all previously allocated strings on failure.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-04-29 11:06:20 +02:00
Wim Taymans
ec04c4bf9a security: add missing NULL check after strdup in pw-dump
Memory Safety: Medium

In the registry event handler, strdup(type) was not checked for
failure. A NULL o->type would cause NULL pointer dereferences in
subsequent code that uses the type string for comparison and logging.

Fix by checking the strdup() return value and cleaning up on failure.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-04-29 11:06:18 +02:00
Wim Taymans
43931caccb security: add missing NULL check after strdup in context factory registry
Memory Safety: Medium

In pw_context_set_spa_libs(), strdup(lib) was not checked for failure.
A NULL entry->lib would cause a NULL dereference when the factory
library path is later looked up and used for dlopen().

Fix by checking the strdup() return value and cleaning up the regex
and array entry on failure.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-04-29 11:06:16 +02:00
Wim Taymans
640af6b20f security: add missing NULL checks after strdup in data-loop
Memory Safety: Medium

In pw_data_loop construction, strdup() calls for the thread affinity
and class strings were not checked for failure. A failed strdup()
would store NULL, leading to NULL pointer dereferences when these
strings are later used for thread configuration.

Fix by checking strdup() return values and failing initialization
with -ENOMEM on allocation failure.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-04-29 11:06:11 +02:00
Wim Taymans
382533da96 security: add missing NULL checks after strdup in impl-metadata
Memory Safety: Medium

The set_item() function called strdup() for key, type, and value
without checking the return values. If any strdup() fails due to
memory exhaustion, the NULL pointer would be stored in the item
struct and later dereferenced when the metadata is accessed or
logged.

Fix by checking strdup() return values and cleaning up on failure.
Change set_item() to return an error code so callers can handle
allocation failures.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-04-29 11:06:09 +02:00
Wim Taymans
eaaf125d13 filter-graph: protect against large values
Limit the delay in the convolver to 10 seconds.

Limit the convolver block sizes to 64K.

Avoid overflows when using large rates, file size or number of
channels in the provided impulse response.
2026-04-29 11:02:11 +02:00
Wim Taymans
72b9577d3c filter: avoid losing buffers in some cases
If the filter process doesn't dequeue/queue a buffer (as can be the
case in jack-tunnel-sink under xrun cases), pw-filter will set the
io to NEED_DATA with ID_INVALID.

This will then make the mixer in the next cycle not recycle any buffers
and it won't be able to produce any new ones either.
If the filter the dequeues/queues a buffer in the next process, it won't
dequeue a buffer for recycle because io is NEED_DATA/INVALID from the
previous cycle (io != HAVE_DATA -> continue).

This will the continue in an infinite loop producing "out of buffers"
forever.

Also check that we actually have a buffer to recycle, if we don't we can
try to dequeue one and place that in the io. This will then unlock the
loop, make the mixer recycle the buffer and produce a new one.

This is the same logic as is present in pw-stream for the same reason.

Fixes #5246
Maybe also #3547
2026-04-28 14:55:13 +02:00