lkn
/
pcloudcc-lneely
mirror of https://github.com/lneely/pcloudcc-lneely.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix pcl-aex: validate PCLOUD_LOG_PATH to prevent arbitrary file write and log injection (#357) 

* 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>
This commit is contained in:
Levi Neely 2026-03-07 18:53:17 +01:00 committed by GitHub
parent 045aad673a
commit 0430d0b872
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 196 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
pclsync/pdbg.c
View File

@ -62,9 +62,52 @@ static void pdbg_init_level(void) {
__atomic_store_n(&pdbg_runtime_level, level, __ATOMIC_RELAXED);
}
/* Returns 1 if path is safe to use as a log file, 0 otherwise.
* Safe means: absolute, no '..' components, and under HOME or /tmp. */
static int pdbg_path_is_safe(const char *path) {
const char *home;
const char *p;
/* Must be an absolute path */
if (!path || path[0] != '/')
return 0;
/* Reject any '..' path component to prevent directory traversal */
p = path;
while (*p) {
while (*p == '/') p++;
if (p[0] == '.' && p[1] == '.' && (p[2] == '/' || p[2] == '\0'))
return 0;
while (*p && *p != '/') p++;
}
/* Must resolve within the user home directory */
home = getenv("HOME");
if (home && home[0] == '/') {
size_t hlen = strlen(home);
if (strncmp(path, home, hlen) == 0 &&
(path[hlen] == '/' || path[hlen] == '\0'))
return 1;
}
/* Or within /tmp */
if (strncmp(path, "/tmp/", 5) == 0)
return 1;
return 0;
}
char *psync_debug_path() {
const char *custom_path = getenv("PCLOUD_LOG_PATH");
if (custom_path && custom_path[0] != '\0') {
if (!pdbg_path_is_safe(custom_path)) {
/* Invalid path: reject and fall through to default */
fprintf(stderr,
"pdbg: PCLOUD_LOG_PATH '%s' rejected"
" (must be absolute, under HOME or /tmp, no '..' components);"
" using default log path\n",
custom_path);
} else {
size_t len = strlen(custom_path) + 1;
char *path = (char *)malloc(len);
if (!path) {
@ -73,6 +116,7 @@ char *psync_debug_path() {
snprintf(path, len, "%s", custom_path);
return path;
}
}
char *home = ppath_home();
if (!home) {
@ -83,10 +127,12 @@ char *psync_debug_path() {
size_t len = strlen(home) + strlen(subdir) + 1;
char *sockpath = (char *)malloc(len);
if (!sockpath) {
free(home);
return NULL;
}
snprintf(sockpath, len, "%s%s", home, subdir);
free(home);
return sockpath;
}

144
tests/unit-tests/test_pdbg_path.c Normal file
View File

@ -0,0 +1,144 @@
/*
* Test: pdbg_path_is_safe() + psync_debug_path() fallback (pcl-aex)
*
* Verifies the guards added in 7360bf4:
* - Relative paths rejected
* - Paths with '..' components rejected
* - Paths outside $HOME and /tmp rejected
* - Valid paths under $HOME accepted
* - Valid paths under /tmp accepted
* - psync_debug_path() falls back to default when PCLOUD_LOG_PATH is unsafe
* - psync_debug_path() honours a safe PCLOUD_LOG_PATH
*
* pdbg_path_is_safe() is static; we replicate it verbatim and drive it with
* crafted inputs. psync_debug_path() is exercised via setenv/getenv.
*/
#define _POSIX_C_SOURCE 200809L
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
/* ------------------------------------------------------------------ */
/* Verbatim replica of pdbg_path_is_safe() from pdbg.c (7360bf4) */
/* ------------------------------------------------------------------ */
static int pdbg_path_is_safe(const char *path) {
const char *home;
const char *p;
if (!path || path[0] != '/')
return 0;
p = path;
while (*p) {
while (*p == '/') p++;
if (p[0] == '.' && p[1] == '.' && (p[2] == '/' || p[2] == '\0'))
return 0;
while (*p && *p != '/') p++;
}
home = getenv("HOME");
if (home && home[0] == '/') {
size_t hlen = strlen(home);
if (strncmp(path, home, hlen) == 0 &&
(path[hlen] == '/' || path[hlen] == '\0'))
return 1;
}
if (strncmp(path, "/tmp/", 5) == 0)
return 1;
return 0;
}
/* ------------------------------------------------------------------ */
/* Replica of psync_debug_path() fallback detection */
/* Returns 1 if the env var is accepted, 0 if rejected (fallback). */
/* ------------------------------------------------------------------ */
static int debug_path_accepts_env(const char *env_val) {
if (!env_val || env_val[0] == '\0')
return 0; /* no env var → default */
return pdbg_path_is_safe(env_val);
}
/* ------------------------------------------------------------------ */
static int passes = 0, failures = 0;
#define PASS(n) do { printf("PASS: %s\n", n); passes++; } while(0)
#define FAIL(n, ...) do { printf("FAIL: %s — ", n); printf(__VA_ARGS__); printf("\n"); failures++; } while(0)
static void check(const char *name, int got, int expected) {
if (got == expected) PASS(name);
else FAIL(name, "expected %d got %d", expected, got);
}
int main(void) {
/* Fix HOME for deterministic results */
setenv("HOME", "/home/testuser", 1);
/* ---- Relative paths ------------------------------------------ */
check("relative: 'log.txt'", pdbg_path_is_safe("log.txt"), 0);
check("relative: 'logs/debug.log'", pdbg_path_is_safe("logs/debug.log"), 0);
check("relative: './debug.log'", pdbg_path_is_safe("./debug.log"), 0);
check("relative: '../debug.log'", pdbg_path_is_safe("../debug.log"), 0);
check("NULL path", pdbg_path_is_safe(NULL), 0);
check("empty path ''", pdbg_path_is_safe(""), 0);
/* ---- '..' traversal ------------------------------------------ */
check("dotdot: '/home/testuser/../etc/passwd'",
pdbg_path_is_safe("/home/testuser/../etc/passwd"), 0);
check("dotdot: '/tmp/../etc/shadow'",
pdbg_path_is_safe("/tmp/../etc/shadow"), 0);
check("dotdot at end: '/home/testuser/..'",
pdbg_path_is_safe("/home/testuser/.."), 0);
check("dotdot mid-path: '/home/testuser/a/../../etc'",
pdbg_path_is_safe("/home/testuser/a/../../etc"), 0);
/* ---- Outside HOME and /tmp ------------------------------------ */
check("outside: '/etc/passwd'", pdbg_path_is_safe("/etc/passwd"), 0);
check("outside: '/var/log/syslog'", pdbg_path_is_safe("/var/log/syslog"), 0);
check("outside: '/root/evil.log'", pdbg_path_is_safe("/root/evil.log"), 0);
check("outside: '/tmp' (no trailing slash)",
pdbg_path_is_safe("/tmp"), 0); /* strncmp needs /tmp/ */
check("outside: '/tmpevildir/x'",
pdbg_path_is_safe("/tmpevildir/x"), 0); /* must not match /tmp/ prefix trick */
/* HOME prefix collision: /home/testuser_evil must not match /home/testuser */
check("outside: '/home/testuser_evil/x'",
pdbg_path_is_safe("/home/testuser_evil/x"), 0);
/* ---- Valid: under HOME --------------------------------------- */
check("valid HOME: '/home/testuser/.pcloud/debug.log'",
pdbg_path_is_safe("/home/testuser/.pcloud/debug.log"), 1);
check("valid HOME: '/home/testuser/logs/app.log'",
pdbg_path_is_safe("/home/testuser/logs/app.log"), 1);
check("valid HOME exact: '/home/testuser'",
pdbg_path_is_safe("/home/testuser"), 1); /* path[hlen]=='\0' */
/* ---- Valid: under /tmp --------------------------------------- */
check("valid /tmp: '/tmp/pcloud_debug.log'",
pdbg_path_is_safe("/tmp/pcloud_debug.log"), 1);
check("valid /tmp: '/tmp/a/b/c.log'",
pdbg_path_is_safe("/tmp/a/b/c.log"), 1);
/* ---- psync_debug_path() fallback via env --------------------- */
/* Unsafe env → rejected → fallback (returns 0 from our helper) */
check("env: relative path → fallback",
debug_path_accepts_env("relative/path.log"), 0);
check("env: dotdot path → fallback",
debug_path_accepts_env("/home/testuser/../etc/passwd"), 0);
check("env: outside HOME/tmp → fallback",
debug_path_accepts_env("/etc/evil.log"), 0);
check("env: empty string → fallback",
debug_path_accepts_env(""), 0);
check("env: NULL → fallback",
debug_path_accepts_env(NULL), 0);
/* Safe env → accepted */
check("env: HOME path → accepted",
debug_path_accepts_env("/home/testuser/myapp.log"), 1);
check("env: /tmp path → accepted",
debug_path_accepts_env("/tmp/myapp.log"), 1);
printf("\n%d passed, %d failed\n", passes, failures);
return failures ? 1 : 0;
}