cli: restrict copy and erase commands

This is a follow-up to PR #717 where path traversal protection was
discussed.  A year later and it's clear that having a user-friendly
copy tool in the shell is a good thing, but that we proably want to
restrict what it can do when called from the CLI.

A sanitize flag (-s) is added to control the behavior, when used in the
shell without -s, both commands act like traditional UNIX tools and do
assume . for relative paths, and allow ../, whereas when running from
the CLI only /media/ is allowed and otherwise files are assumed to be
in $HOME or /cfg

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>

Author: troglobit

src/bin/copy.c

@@ -36,6 +36,7 @@ static const char *prognm = "copy";
 static int timeout;
 static int dry_run;
 static int quiet;
+static int sanitize;
 
 
 /*
@@ -205,7 +206,7 @@ static int replace_running(sr_conn_ctx_t *conn, sr_session_ctx_t *sess,
 
 	if (dry_run) {
 		if (!quiet) {
-			printf("Configuration validated, %s: OK\n", file);
+			printf("Configuration validated, %s OK\n", file);
 			fflush(stdout);
 		}
 		lyd_free_all(data);
@@ -335,10 +336,10 @@ static int copy(const char *src, const char *dst, const char *remote_user)
 		if (dstds && dstds->path)
 			fn = dstds->path;
 		else
-			fn = cfg_adjust(dst, src, adjust, sizeof(adjust));
+			fn = cfg_adjust(dst, src, adjust, sizeof(adjust), sanitize);
 
 		if (!fn) {
-			fprintf(stderr, ERRMSG "invalid destination path.\n");
+			fprintf(stderr, ERRMSG "file not found.\n");
 			rc = -1;
 			goto err;
 		}
@@ -372,9 +373,9 @@ static int copy(const char *src, const char *dst, const char *remote_user)
 			tmpfn = mktemp(temp_file);
 			fn = tmpfn;
 		} else {
-			fn = cfg_adjust(src, NULL, adjust, sizeof(adjust));
+			fn = cfg_adjust(src, NULL, adjust, sizeof(adjust), sanitize);
 			if (!fn) {
-				fprintf(stderr, ERRMSG "invalid source file location.\n");
+				fprintf(stderr, ERRMSG "file not found.\n");
 				rc = 1;
 				goto err;
 			}
@@ -416,9 +417,9 @@ static int copy(const char *src, const char *dst, const char *remote_user)
 		}
 
 		if (strstr(src, "://")) {
-			fn = cfg_adjust(dst, src, adjust, sizeof(adjust));
+			fn = cfg_adjust(dst, src, adjust, sizeof(adjust), sanitize);
 			if (!fn) {
-				fprintf(stderr, ERRMSG "invalid destination file location.\n");
+				fprintf(stderr, ERRMSG "file not found.\n");
 				rc = 1;
 				goto err;
 			}
@@ -432,9 +433,9 @@ static int copy(const char *src, const char *dst, const char *remote_user)
 
 			rc = systemf("curl %s -Lo %s %s", remote_user, fn, src);
 		} else if (strstr(dst, "://")) {
-			fn = cfg_adjust(src, NULL, adjust, sizeof(adjust));
+			fn = cfg_adjust(src, NULL, adjust, sizeof(adjust), sanitize);
 			if (!fn) {
-				fprintf(stderr, ERRMSG "invalid source file location.\n");
+				fprintf(stderr, ERRMSG "file not found.\n");
 				rc = 1;
 				goto err;
 			}
@@ -472,6 +473,7 @@ static int usage(int rc)
 	       "  -h         This help text\n"
 	       "  -n         Dry-run, validate configuration without applying\n"
 	       "  -q         Quiet mode, suppress informational messages\n"
+	       "  -s         Sanitize paths for CLI use (restrict path traversal)\n"
 	       "  -u USER    Username for remote commands, like scp\n"
 	       "  -t SEEC    Timeout for the operation, or default %d sec\n"
 	       "  -v         Show version\n", prognm, timeout);
@@ -486,7 +488,7 @@ int main(int argc, char *argv[])
 
 	timeout = fgetint("/etc/default/confd", "=", "CONFD_TIMEOUT");
 
-	while ((c = getopt(argc, argv, "hnqt:u:v")) != EOF) {
+	while ((c = getopt(argc, argv, "hnqst:u:v")) != EOF) {
 		switch(c) {
 		case 'h':
 			return usage(0);
@@ -496,6 +498,9 @@ int main(int argc, char *argv[])
 		case 'q':
 			quiet = 1;
 			break;
+		case 's':
+			sanitize = 1;
+			break;
 		case 't':
 			timeout = atoi(optarg);
 			break;

src/bin/erase.c

@@ -10,7 +10,7 @@
 #include "util.h"
 
 static const char *prognm = "erase";
-
+static int sanitize;
 
 static int do_erase(const char *path)
 {
@@ -25,7 +25,7 @@ static int do_erase(const char *path)
 			return -1;
 		}
 
-		cfg_adjust(path, NULL, fn, len);
+		cfg_adjust(path, NULL, fn, len, sanitize);
 	} else
 		fn = (char *)path;
 
@@ -46,6 +46,7 @@ static int usage(int rc)
 	       "\n"
 	       "Options:\n"
 	       "  -h         This help text\n"
+	       "  -s         Sanitize paths for CLI use (restrict path traversal)\n"
 	       "  -v         Show version\n", prognm);
 
 	return rc;
@@ -55,10 +56,13 @@ int main(int argc, char *argv[])
 {
 	int c;
 
-	while ((c = getopt(argc, argv, "hv")) != EOF) {
+	while ((c = getopt(argc, argv, "hsv")) != EOF) {
 		switch(c) {
 		case 'h':
 			return usage(0);
+		case 's':
+			sanitize = 1;
+			break;
 		case 'v':
 			puts(PACKAGE_VERSION);
 			return 0;

src/bin/util.c

@@ -5,6 +5,7 @@
 #include <errno.h>
 #include <stdarg.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 #include <termios.h>
 
@@ -81,38 +82,84 @@ static const char *basenm(const char *fn)
 	return ptr;
 }
 
-char *cfg_adjust(const char *fn, const char *tmpl, char *buf, size_t len)
+char *cfg_adjust(const char *fn, const char *tmpl, char *buf, size_t len, int sanitize)
 {
-	if (strstr(fn, "../"))
-		return NULL;	/* relative paths not allowed */
-
-	if (fn[0] == '/') {
-		strlcpy(buf, fn, len);
-		return buf;	/* allow absolute paths */
-	}
-
-	/* Files in /cfg must end in .cfg */
-	if (!strncmp(fn, "/cfg/", 5)) {
+	if (sanitize) {
+		char testpath[PATH_MAX];
+		/* CLI mode - restricted and helpful */
+
+		/* Block path traversal attacks */
+		if (strstr(fn, "../"))
+			return NULL;
+
+		/* Handle absolute paths */
+		if (fn[0] == '/') {
+			/* Only allow /media/ prefix for USB sticks */
+			if (strncmp(fn, "/media/", 7) == 0) {
+				strlcpy(buf, fn, len);
+				return buf;
+			}
+			/* Block other absolute paths */
+			return NULL;
+		}
+
+		/* Relative paths: try CWD first (user's $HOME), then /cfg */
+
+		/* Check if file exists in current directory */
+		if (access(fn, F_OK) == 0) {
+			/* Resolve to absolute path from CWD */
+			if (realpath(fn, buf))
+				return buf;
+			/* If realpath fails, use as-is */
+			strlcpy(buf, fn, len);
+			return buf;
+		}
+
+		/* Not in CWD, try /cfg with .cfg extension */
+		snprintf(testpath, sizeof(testpath), "/cfg/%s", fn);
+		if (!has_ext(testpath, ".cfg"))
+			strlcat(testpath, ".cfg", sizeof(testpath));
+
+		if (access(testpath, F_OK) == 0) {
+			strlcpy(buf, testpath, len);
+			return buf;
+		}
+
+		/* Try /cfg without adding extension */
+		snprintf(testpath, sizeof(testpath), "/cfg/%s", fn);
+		if (access(testpath, F_OK) == 0) {
+			strlcpy(buf, testpath, len);
+			return buf;
+		}
+
+		/* File not found in either location, try CWD anyway (for new files) */
 		strlcpy(buf, fn, len);
-		if (!has_ext(fn, ".cfg"))
-			strlcat(buf, ".cfg", len);
-
 		return buf;
+
 	}
 
-	/* Files ending with .cfg belong in /cfg */
-	if (has_ext(fn, ".cfg")) {
-		snprintf(buf, len, "/cfg/%s", fn);
+	/* Shell mode - standard UNIX filesystem semantics */
+
+	/* Allow all absolute paths */
+	if (fn[0] == '/') {
+		strlcpy(buf, fn, len);
 		return buf;
 	}
 
-	if (strlen(fn) > 0 && fn[0] == '.' && tmpl) {
+	/* Relative paths - resolve from CWD, no magic */
+	if (strlen(fn) > 0 && fn[0] == '.' && fn[1] == '/' && tmpl) {
+		/* Handle ./ prefix explicitly */
+		strlcpy(buf, fn, len);
+	} else if (strlen(fn) > 0 && fn[0] == '.' && tmpl) {
+		/* Handle . or .. with template */
 		if (fn[1] == '/' && fn[2] != 0)
 			strlcpy(buf, fn, len);
 		else
 			strlcpy(buf, basenm(tmpl), len);
-	} else
+	} else {
+		/* Plain relative path - use as-is */
 		strlcpy(buf, fn, len);
+	}
 
 	return buf;
 }

src/bin/util.h

@@ -12,6 +12,6 @@ int   yorn       (const char *fmt, ...);
 int   files      (const char *path, const char *stripext);
 
 int   has_ext    (const char *fn, const char *ext);
-char *cfg_adjust (const char *fn, const char *tmpl, char *buf, size_t len);
+char *cfg_adjust (const char *fn, const char *tmpl, char *buf, size_t len, int sanitize);
 
 #endif /* BIN_UTIL_H_ */

src/klish-plugin-infix/src/infix.c

@@ -120,7 +120,7 @@ int infix_erase(kcontext_t *ctx)
 
 	cd_home(ctx);
 
-	return systemf("erase %s", path);
+	return systemf("erase -s %s", path);
 }
 
 int infix_files(kcontext_t *ctx)
@@ -221,7 +221,7 @@ int infix_copy(kcontext_t *ctx)
 		strlcpy(validate, "-n", sizeof(validate));
 
 	/* Ensure we run the copy command as the logged-in user, not root (klishd) */
-	return systemf("doas -u %s copy %s %s %s %s", cd_home(ctx), validate, user, src, dst);
+	return systemf("doas -u %s copy -s %s %s %s %s", cd_home(ctx), validate, user, src, dst);
 }
 
 int infix_shell(kcontext_t *ctx)