The decrypt() function for crypto_box (public-key) used
crypto_secretbox_NONCEBYTES and crypto_secretbox_MACBYTES
(secret-key constants) instead of crypto_box_* constants.
These happen to be equal in libsodium but are semantically
different APIs. Fix ensures consistency with the encrypt()
counterpart.
piDecompress() appended original size as 8-byte ullong at the end
of compressed data, but passed zba.size() (including the 8 bytes)
to uncompress(). This fed trailing metadata as part of the zlib
stream. Fix: subtract sizeof(ullong) from source length.
received() acquired mutex_header and mutex_session in different orders
depending on packet type:
- pt_Data path: mutex_header → mutex_session
- pt_Start path: mutex_session → mutex_header
send_process() acquires mutex_session independently. When running
concurrently with received(), the reversed lock ordering creates a
classic ABBA deadlock scenario.
Fix: enforce consistent ordering (mutex_header → mutex_session) in
all code paths. Restructured pt_Start case and buildSession() to
always acquire mutex_header before mutex_session.
The background run() thread copies allConnections() then iterates
each connection's diags_ map. If a PIConnection is deleted from
another thread between the copy and the iteration, dereferencing
the pointer is UAF. Add null check and protect diags_ iteration
with __device_pool__ lock.
Flags break_, is_sending, is_receiving, is_pause are accessed from
both the send thread (send_process) and the read thread (received)
without synchronization. Plain bool reads/writes from multiple threads
is undefined behavior per C++ standard. Convert to std::atomic<bool>
with proper initializers.
piMini(max_size, buff.size()) implicitly narrows size_t to int,
which is UB for large buffers. Replace with explicit piMin<ssize_t>
matching the correct pattern used in picloudclient.cpp.
The buffer was sized as floor(sz/4)*3 but the loop ran ceil(sz/4)
times, writing 3 bytes per iteration. For sz%4!=0, this wrote past
the buffer end. Also guarded sz<4 to avoid processing trivially
short or malformed input.
The function called unlock() on thread_mutex without a matching
lock(), causing UB on Windows (releasing an unowned critical
section) or silently dropping a user-held lock.
When elapsed time exceeded the timeout, the remaining milliseconds
expression went negative and was implicitly converted to DWORD,
wrapping to ~0xFFFFFFFF (5 days) — effectively INFINITE.
Now clamps to 0 and returns false when time has elapsed.
dirent** list was uninitialized, so scandir failure (returning -1)
left it with indeterminate value. The unconditional free(list) at
the end was then UB — typically heap corruption or crash.
pop_back() called resize(size_ - 1) without checking for empty.
On empty array, uint underflow produced UINT_MAX, causing
bytesInBits(UINT_MAX) overflow and subsequent OOB access.
pop_front() already had this guard; pop_back() was missing it.
Three complex type cases (pivComplexf, pivComplexd, pivComplexld)
lacked break statements, causing fallthrough to pivTime. Setting
a complex variant from string silently overwrote the value with
PITime::fromString(), corrupting the variant's type and data.
When mmap() returned MAP_FAILED, data stayed as (void*)-1, causing
subsequent read/write/close to pass the null guard and crash.
Now resets data=nullptr, and calls shm_unlink when owner=true to
avoid orphaned shared memory objects.
The float FFT plan generator wrote 8 ints per entry (indices +0..+7)
but declared entrysize=4, causing heap overwrite into the next entry
and incorrect plan array sizing.
processFile() accessed files_[id - 1] without validating id.
When id == 0 (from a crafted network packet), id - 1 = -1 caused
out-of-bounds access. Added bounds check: id must be in range
[1, files_.size()]. Stops receive and logs error on invalid id.
fork() can return -1 on error (too many processes, out of memory).
The old code treated pid_ == -1 as a successful fork, entering the
parent branch and calling waitpid(-1, ...) which waits for ANY
child process. Added explicit check: close pipes, free memory,
and return on fork failure.
In the blocking read path (timeout_ms <= 0), the while loop had no
exit condition for readDevice() returning 0 (EOF) or -1 (error).
This caused an infinite busy-loop at 100% CPU. Added 'else break'
to exit the loop on read failure, matching the timeout branch
which already handles this case.
readDevice() did pos += size instead of pos += ret. The clamping
on the next line (pos = min(pos, data_->size_s())) masked the
issue in practice, but the semantics were wrong. Aligns with
writeDevice() which correctly uses pos += rs.size_s().
open() returns -1 on error. The check fd != 0 treated -1 as a
valid fd (since -1 != 0 is true), causing devctl(-1, ...) and
close(-1) on QNX. Fixed to fd >= 0.
File descriptor 0 is stdin (a valid fd). Using 0 as the 'not-open'
sentinel caused isCreate() to return false and destroy() to skip
close() when pipe() happened to allocate fds {0, 1} (e.g. in daemons
with closed stdio). POSIX convention: -1 = invalid fd.
When the custom client_factory() returned nullptr, the incoming
PIEthernet pointer was leaked. Matches existing pattern used in the
max_clients overflow branch.
- toChar(): char v was uninitialized, causing UB when
sscanf fails to match (empty string). Initialize to 0.
- operator+=(PIConstChars): loop iterated l < d.size() instead of
l < str.size(), reading past the end of str after d.enlarge().
This is a heap buffer overread with undefined behavior.
andrey
peri4