Replace fopen() with open()+fdopen() for all log file creation
in pdbg.c to ensure files are created with mode 0600, preventing
world-readable logs that may contain sensitive information.
Fixes#290
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
* Fix pcl-dls.1: free errPtr between calls in ptools_set_backend_file_dates()
char *errPtr was already used instead of char msgErr[1024], but was
not freed between the two ptools_backend_call() invocations. If the
first call allocated errPtr, the second would overwrite the pointer
without freeing it. Add free(errPtr); errPtr = NULL; between the two
calls to eliminate the leak.
Ref GH #194.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Add ptools_set_backend_file_dates() errPtr lifecycle tests (pcl-dls)
6 test cases using malloc/free wrapping covering both-succeed,
call1-error, call2-error, both-errors, pre-fix leak demonstration,
and no-double-free after mid-free NULLing.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* Fix pcl-aex.1: validate PCLOUD_LOG_PATH before use in psync_debug_path()
Add pdbg_path_is_safe() helper that rejects PCLOUD_LOG_PATH values
that are not absolute, contain '..' path components, or do not resolve
under the user HOME directory or /tmp. On rejection, fall back to the
default ~/.pcloud/debug.log path and emit a warning to stderr.
Also fix a pre-existing memory leak: ppath_home() return was not freed
in the default-path branch.
Ref GH #291.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Add PCLOUD_LOG_PATH path-safety tests (pcl-aex)
28 test cases covering relative paths, '..' traversal, paths outside
HOME and /tmp, valid accepted paths, and psync_debug_path() env
fallback behaviour.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* Fix pcl-aqb.1: eliminate function-pointer cast UB in start_thread() via union
Replace the single void* run field in thread_data with a union
{ thread0_run run0; thread1_run run1 } so each function pointer is
stored and retrieved at its correct type, eliminating the
thread0_run <-> thread1_run cast chain UB. Split start_thread() into
prun_thread() and prun_thread1() that each populate the appropriate
union member, backed by a shared start_thread_common() helper. Add
malloc NULL check with error log in both public functions.
pthread_create failure handling (log + free) was already present.
Ref GH #199.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Add prun start_thread() resource-leak and UB tests (pcl-aqb)
8 test cases using linker interposition covering pthread_create
failure data-free, attr_destroy on failure, malloc failure graceful
return, union fn storage without cast, and success path accounting.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* Fix pcl-a1j.1: replace sprintf with snprintf and add strcat length check in ptools_create_backend_event()
Add paramname length check (> 254 bytes → skip with warning) before
snprintf into charBuff[i][258], and validate the snprintf return
value. Add explicit length check before strcat into keyParams to
prevent overflow when paramname exceeds remaining buffer space.
Eliminates buffer overflow from long paramname.
Ref GH #195.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Fix pcl-a1j.1: clamp pCnt to PTOOLS_MAX_PARAMS to prevent charBuff stack overflow
charBuff[30][258] is a fixed-size stack array but pCnt was unbounded,
allowing any caller with params->paramCnt > 30 to overflow the stack
via charBuff[i] access. Add PTOOLS_MAX_PARAMS (30) define, use it to
size charBuff, and clamp pCnt to PTOOLS_MAX_PARAMS with a warning log
before the loop.
Ref GH #195.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Add ptools_create_backend_event() validation tests (pcl-a1j)
11 test cases covering pCnt clamping, paramname length guards,
snprintf boundary, keyParams overflow check, and comma-prefix
for subsequent params.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* Fix pcl-3la.1: saturating addition for bytestou in pstatus_upload_recalc
Replace unchecked uint64_t summation in pstatus_upload_recalc() with
overflow-safe saturating addition. Before adding each file's size,
check if the result would wrap around UINT64_MAX; if so, saturate to
UINT64_MAX instead of silently wrapping.
Ref GH #293.
* Fix pcl-3la.2: remove spurious fuse_loop_mt while-loop and exit(0)
fuse_loop_mt() already loops internally; wrapping it in an additional
while loop caused repeated re-entry after normal exit. Also removes
the exit(0) call in the shutdown path so fuse_destroy() and other
cleanup run properly on shutdown.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace indefinite pthread_cond_wait() calls in pstatus_wait() with
pthread_cond_timedwait() using a 5-second timeout computed via
clock_gettime(CLOCK_REALTIME). On timeout, the while loop re-evaluates
the wait condition, preventing indefinite hang when the state machine
stalls.
Ref GH #292.
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
- Use sig_atomic_t flag instead of calling unsafe functions in signal handlers
- Remove pdbg_logf() and exit() calls from psync_signal_handler
- Check shutdown_requested flag in main loops
- Export flag for cross-module access
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
- connect_res(): add SO_ERROR check after EINPROGRESS + wait_writable;
a refused/reset connection makes the socket writable with SO_ERROR
set — previously returned the socket as 'connected' when it was not.
Added psock_check_so_error() helper using getsockopt(SO_ERROR).
- wait_readable(), wait_writable(), wait_ssl_ready(): wrap select() in
do{...}while(errno==EINTR) with fd_set reinit on each iteration.
Signal delivery during blocking waits previously caused immediate
SOCKET_ERROR return, dropping live connections on any signal.
Linux updates tv on EINTR to reflect remaining time; reinitializing
fd_set before each retry ensures a clean select state.
- psync_socket_read_plain(), psync_socket_readall_plain(),
psync_socket_readall_plain_thread(): add EINTR to EAGAIN/EWOULDBLOCK
retry condition on read(). EINTR is a non-fatal interruption; retrying
is correct and consistent with the non-blocking socket model.
- psync_socket_read_noblock_plain(): return PSYNC_SOCKET_WOULDBLOCK on
EINTR rather than -1 (fatal error); callers handle WOULDBLOCK by
scheduling a retry.
- psync_socket_writeall_plain(), psync_socket_writeall_plain_thread():
add EINTR retry path on write() without a write-timeout wait, since
EINTR does not indicate the socket became unwritable.
- psock_connect(): add null check on malloc(sizeof(psock_t)); on OOM,
close the connected fd and free the SSL context before returning NULL.
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
prpc.c:
- on_request: fix total_size computation. The original formula
sizeof(uint32_t)+sizeof(uint64_t)+response->length had two bugs:
(1) sizeof(uint32_t)+sizeof(uint64_t)=12 but offsetof(rpc_message_t,value)=16
due to struct alignment padding between the uint32_t type and uint64_t length
fields; and (2) respond() was storing full message size in response->length,
double-counting the header. Fix: use offsetof(rpc_message_t,value)+response->length,
consistent with readResponse() in rpcclient.cpp which reads a fixed header_size
of offsetof(rpc_message_t,value) bytes then reads msg->length payload bytes.
- respond: store payload length only in response->length (value_length+1),
not full message size, to match the client protocol expectation.
- prpc_init: add null check on malloc return value.
- prpc_register: restore old handler table and return -1 if prpc_init fails.
papi.c:
- papi_result_thread: add missing MAX_API_RESPONSE_SIZE guard (present in
papi_result but absent here), preventing server-controlled unbounded malloc.
- papi_result, papi_result_thread: add null checks on malloc before passing
pointer to psock_readall.
- papi_result_async: add MAX_API_RESPONSE_SIZE check and malloc null check
on reader->respsize path.
- calc_ret_len: add _NEED_DATA(1) guard before ARRAY and HASH while-loop
conditions; empty containers with datalen=0 caused out-of-bounds read.
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
status_change_thread reads psync_status.filestodownload, filestoupload,
localisfull, and remoteisfull under statusmutex, while pstatus_download_recalc,
pstatus_upload_recalc, and pstatus_set write them without statusmutex (or under
a different mutex). TSan reports the race at pqevent.c:274.
Replace all writes of these four fields with __atomic_store_n and capture
atomic snapshots in status_change_thread before the condition, using the
snapshots for both comparison and the status_old update after struct copy.
Also use atomic loads in calc_status() for filestodownload and filestoupload.
Closes#335
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
psync_status.status is written in pstatus_set() (pstatus.c:263) after
releasing status_internal_mutex, while status_change() (pclsync_lib.cpp:435)
reads it concurrently from the callback thread without any lock. TSan
reports the race between T9 (write in pstatus_set) and the status callback
thread (read in status_change).
Replace all reads and writes of psync_status.status with
__atomic_load_n/__atomic_store_n (__ATOMIC_RELAXED) across pstatus.c,
pqevent.c, and pclsync_lib.cpp. In status_change_thread, capture the
atomic value once into cur_status before the condition to avoid multiple
inconsistent loads. In status_change(), capture cur_status at entry and
use it throughout, also propagating it to the copied status_ struct.
Closes#334
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
psync_current_time is a global time_t written by timer_thread without
holding timer_mutex, while ptimer_register reads it under timer_mutex.
TSan reports the race at ptimer.c:152 (write) and ptimer.c:216 (read).
Replace all reads with __atomic_load_n and all writes with
__atomic_store_n using __ATOMIC_RELAXED. This covers the race sites in
ptimer.c and all external readers in plocalscan.c, pnetlibs.c, and
psynclib.c that access psync_current_time without any mutex.
Closes#333
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Acquire mutex before reading tm->refcnt to prevent race with
psync_task_dec_refcnt(). Fixes ThreadSanitizer warning.
Fixes#332
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
In psync_unlink(), putil_wipe(psync_my_pass, sizeof(psync_my_pass))
only wiped 8 bytes (pointer size) instead of the full password string.
Changed to strlen(psync_my_pass) and added NULL check.
Fixes pcl-zqv.9.1 (partial)
GH #276
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
strcmp() at line 315 short-circuits on first differing byte, leaking
timing info about trusted fingerprints.
Replace with constant-time comparison using memcmp and bitwise OR
accumulation across all trusted entries.
Fixes GH #239
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
reader->data = malloc(reader->respsize) at line 462 is not checked
for NULL before goto again continues the loop and dereferences it.
Add NULL check and return ASYNC_RES_READY with result=NULL on failure.
Fixes GH #238
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
cbidx = request->type - 20 computed before checking lower bound.
If request->type < calbacks_lower_band, cbidx wraps to large value
causing out-of-bounds array access.
Move bounds check before subtraction and use calbacks_lower_band
instead of hardcoded 20.
Fixes GH #235
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
msgErr was declared as char[1024] but cast to char** and passed to
ptools_backend_call(). The function writes a char* into *err,
corrupting the first sizeof(char*) bytes of the array.
Change to char *errPtr = NULL and free it after use.
Fixes GH #216
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Loop variable j was declared inside the loop and reset to 0 on every
iteration. All branches had early continue, so j++ never executed.
Change j to i - reqParCnt to correctly index into optionalParams array.
Also fix paramtype check to use j instead of i.
Fixes GH #215
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
When eventid has PEVENT_TYPE_FOLDER set, the code allocated
psync_file_event_t size but cast to psync_folder_event_t.
Swap the sizeof() calls to match the actual struct types used.
Fixes GH #229
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Three functions had a consistent typo (missing 'i' in builder):
- psync_list_bulder_push_num
- psync_list_bulder_pop_num
- psync_list_bulder_add_element
Renamed to correct spelling in plist.h, plist.c, and all callers.
Fixes GH #219
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
excepions is read without holding timer_ex_mutex, but
ptimer_exception_handler() modifies it under the mutex.
Move the read inside the critical section to prevent the race.
Fixes GH #227
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
pthread_create() return value is ignored. If it fails, the allocated
thread_data is leaked.
Check return value; on error, log and free the data.
Fixes GH #221
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
keyParams is only allocated when pCnt > 0 but freed unconditionally.
When pCnt == 0, free() is called on an uninitialized pointer (UB).
Initialize to NULL so free() is a no-op when allocation is skipped.
Fixes GH #214
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
mmap(2) returns MAP_FAILED ((void*)-1) on error, not NULL. The check
'if (likely(ret))' treats MAP_FAILED as truthy, so the emergency retry
path is never reached and callers receive MAP_FAILED as a valid pointer.
Normalize pmem_mmap() to return NULL on failure (converting MAP_FAILED
to NULL in the mmap path). This confines the platform-specific error
handling to one place and eliminates the need for preprocessor checks
in pmem_mmap_safe() and psync_mmap_anon_emergency().
Fixes GH #226
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
The weak/strong symbol approach requires that psql.c undefs the debug
macros to define function implementations. This means functions in psql.c
call psql_lock()/psql_unlock() as functions, not macros.
In debug builds, these must be strong overrides that call the _do_
variants to properly maintain lockctr/rdlockctr. Without these overrides,
the weak stubs are used which don't update counters, causing assertion
failures in psql_unlock().
Added strong overrides in psql_debug.c for:
- psql_lock() -> psql_do_lock(__FILE__, __LINE__)
- psql_rdlock() -> psql_do_rdlock(__FILE__, __LINE__)
- psql_statement() (calls psql_do_lock directly)
Also made psql_statement, psql_start, and all query/prepare functions
weak in psql.c to allow debug overrides.
Fixes#138
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
* Fix#103: Refactor putil to use putil_ namespace and enforce encapsulation
All public functions in putil now use the putil_ prefix (e.g., putil_strdup,
putil_strcat, putil_time_format, putil_base64_encode, etc.). Internal
variables (normalize_table, base64_reverse_table) and the constructor remain
static. Updated all 34 call-site files across pclsync accordingly.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Fix#96: Refactor pfs to use pfs_ namespace and enforce encapsulation
All public functions in the pfs module renamed from psync_fs_ to pfs_
(e.g., pfs_update_openfile, pfs_refresh, pfs_start, pfs_stop, etc.).
Extern vars psync_fake_prefix/psync_fake_prefix_len renamed to
pfs_fake_prefix/pfs_fake_prefix_len. Functions only used within pfs.c
(pfs_update_openfile_fileid_locked, pfs_chown) made static. Updated all
call sites across 17 files including pclsync_lib.cpp.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Refactor pfsxattr to use pfs_xattr_ namespace
All psync_fs_ prefixed functions in the pfsxattr module renamed to
pfs_xattr_ (e.g., pfs_xattr_set, pfs_xattr_get, pfs_xattr_list,
pfs_xattr_remove, pfs_xattr_file_deleted, pfs_xattr_task_to_file, etc.).
Local macro psync_fs_set_thread_name renamed to pfs_xattr_set_thread_name.
Internal static helpers (delete_object_id, update_object_id,
xattr_get_object_id_locked) remain static and unchanged.
Updated all call sites in pfs.c, pfsupload.c, and pdiff.c.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Rename pfs_xattr_ namespace to pfs_xatr_
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Refactor pfs sub-modules to use namespaced function prefixes
- pfscrypto_* → pfs_crpt_*
- psync_fsstatic_* → pfs_stat_*
- psync_fsupload_* → pfs_upld_*
- psync_fstask_* → pfs_task_* (functions only; typedefs unchanged)
- psync_fsfolder_*/psync_fsfolderid_*/psync_fsfolderflags_*/
get_decname_for_folder/psync_get_folderid → pfs_fldr_*
Type names, macro constants, and typedef aliases are unchanged.
Updated all call sites across 17 files.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Levi Neely