From ea81780895702b08b0b93ff48bd1876330632b89 Mon Sep 17 00:00:00 2001 From: Alvar Penning Date: Thu, 5 Feb 2026 20:58:25 +0100 Subject: strip_exif support for the OpenBSD sandbox Change the strip_exif logic to work with the already existing OpenBSD sandbox and allow ffmpeg and mogrify to be executed. The previous strip_exif implementation relied on system(3), effectively starting "/bin/sh" and executing the required tool within a shell session. Making this work in the sandbox would require to allow executing "/bin/sh", rendering the sandbox useless. Thus, the code now starts determining the absolute path of the tools - unless they are given as ffmpeg_path or mogrify_path - and allowing them to be executed via unveil(2). Then, instead of the system(3) call, the good old fork(2) and execve(2) dance is performed. The sbox_enter code was made aware of strip_exif, which resulted in a pledge(2) violation before when disable_email_notifications was set to false. Furthermore, the detected paths of the tools are now allowed. --- data.c | 19 +++----- sandbox.c | 9 +++- snac.c | 158 +++++++++++++++++++++++++++++++++++++++----------------------- snac.h | 1 + 4 files changed, 114 insertions(+), 73 deletions(-) diff --git a/data.c b/data.c index 9616fee..1aac759 100644 --- a/data.c +++ b/data.c @@ -100,19 +100,12 @@ int srv_open(const char *basedir, int auto_upgrade) if (auto_upgrade) ret = snac_upgrade(&error); - else { - if (xs_number_get(xs_dict_get(srv_config, "layout")) < disk_layout) - error = xs_fmt("ERROR: disk layout changed - execute 'snac upgrade' first"); - else { - if (!check_strip_tool()) { - const char *mp = xs_dict_get(srv_config, "mogrify_path"); - if (mp == NULL) mp = "mogrify"; - error = xs_fmt("ERROR: strip_exif enabled but '%s' not found or not working (set 'mogrify_path' in server.json)", mp); - } - else - ret = 1; - } - } + else if (xs_number_get(xs_dict_get(srv_config, "layout")) < disk_layout) + error = xs_fmt("ERROR: disk layout changed - execute 'snac upgrade' first"); + else if (!check_strip_tool()) + error = xs_fmt("ERROR: strip_exif enabled but commands not found or working"); + else + ret = 1; } } diff --git a/sandbox.c b/sandbox.c index c6cfdcb..15e4622 100644 --- a/sandbox.c +++ b/sandbox.c @@ -13,6 +13,8 @@ void sbox_enter(const char *basedir) return; } + const xs_val *strip_exif = xs_dict_get(srv_config, "strip_exif"); + int smail; const char *url = xs_dict_get(srv_config, "smtp_url"); @@ -33,6 +35,11 @@ void sbox_enter(const char *basedir) if (*address == '/') unveil(address, "rwc"); + if (strip_exif) { + unveil(xs_dict_get(srv_config, "ffmpeg_path"), "x"); + unveil(xs_dict_get(srv_config, "mogrify_path"), "x"); + } + if (smail) unveil("/usr/sbin/sendmail", "x"); @@ -45,7 +52,7 @@ void sbox_enter(const char *basedir) if (*address == '/') p = xs_str_cat(p, " unix"); - if (smail) + if (smail || strip_exif) p = xs_str_cat(p, " exec"); pledge(p, NULL); diff --git a/snac.c b/snac.c index edf0a1e..9c84205 100644 --- a/snac.c +++ b/snac.c @@ -35,7 +35,9 @@ #include #include #include +#include #include +#include xs_str *srv_basedir = NULL; xs_dict *srv_config = NULL; @@ -177,6 +179,46 @@ int check_password(const char *uid, const char *passwd, const char *hash) } +char* findprog(const char *prog) +/* find a prog in PATH and return the first match */ +{ + char *path_env, *path, *dir, filename[PATH_MAX]; + int len; + struct stat sbuf; + + path_env = getenv("PATH"); + if (!prog || !path_env) + return NULL; + + path_env = strdup(path_env); + if (!path_env) + return NULL; + path = path_env; + + while ((dir = strsep(&path, ":")) != NULL) { + /* empty entries as ./ instead of / */ + if (*dir == '\0') + dir = "."; + + /* strip trailing / */ + len = strlen(dir); + while (len > 0 && dir[len-1] == '/') + dir[--len] = '\0'; + + len = snprintf(filename, sizeof(filename), "%s/%s", dir, prog); + if (len > 0 && len < (int) sizeof(filename) && + (stat(filename, &sbuf) == 0) && S_ISREG(sbuf.st_mode) && + access(filename, X_OK) == 0) { + free(path_env); + return strdup(filename); + } + } + + free(path_env); + return NULL; +} + + int strip_media(const char *fn) /* strips EXIF data from a file */ { @@ -207,23 +249,24 @@ int strip_media(const char *fn) xs_endswith(l_fn, ".gif") || xs_endswith(l_fn, ".bmp")) { const char *mp = xs_dict_get(srv_config, "mogrify_path"); - if (mp == NULL) - mp = "mogrify"; - xs *cmd = xs_fmt("cd \"%s\" && %s -auto-orient -strip \"%s\" 2>/dev/null", srv_basedir, mp, r_fn); - - ret = system(cmd); + pid_t pid = fork(); + if (pid == -1) { + srv_log(xs_fmt("strip_media: cannot fork()")); + return -1; + } else if (pid == 0) { + chdir(srv_basedir); + execl(mp, "-auto-orient", "-strip", r_fn, (char*) NULL); + _exit(1); + } - if (ret != 0) { - int code = 0; - if (WIFEXITED(ret)) - code = WEXITSTATUS(ret); - - if (code == 127) - srv_log(xs_fmt("strip_media: error stripping %s. '%s' not found (exit 127). Set 'mogrify_path' in server.json.", r_fn, mp)); - else - srv_log(xs_fmt("strip_media: error stripping %s %d", r_fn, ret)); + if (waitpid(pid, &ret, 0) == -1) { + srv_log(xs_fmt("strip_media: cannot waitpid()")); + return -1; } + + if (ret != 0) + srv_log(xs_fmt("strip_media: error stripping %s %d", r_fn, ret)); else srv_debug(1, xs_fmt("strip_media: stripped %s", r_fn)); } @@ -234,35 +277,33 @@ int strip_media(const char *fn) xs_endswith(l_fn, ".mkv") || xs_endswith(l_fn, ".avi")) { const char *fp = xs_dict_get(srv_config, "ffmpeg_path"); - if (fp == NULL) - fp = "ffmpeg"; /* ffmpeg cannot modify in-place, so we need a temp file */ /* we must preserve valid extension for ffmpeg to guess the format */ const char *ext = strrchr(r_fn, '.'); if (ext == NULL) ext = ""; xs *tmp_fn = xs_fmt("%s.tmp%s", r_fn, ext); - - /* -map_metadata -1 strips all global metadata */ - /* -c copy copies input streams without re-encoding */ - /* we don't silence stderr so we can debug issues */ - /* we explicitly cd to srv_basedir to ensure relative paths work */ - xs *cmd = xs_fmt("cd \"%s\" && %s -y -i \"%s\" -map_metadata -1 -c copy \"%s\"", srv_basedir, fp, r_fn, tmp_fn); - ret = system(cmd); + pid_t pid = fork(); + if (pid == -1) { + srv_log(xs_fmt("strip_media: cannot fork()")); + return -1; + } else if (pid == 0) { + chdir(srv_basedir); + /* -map_metadata -1 strips all global metadata */ + /* -c copy copies input streams without re-encoding */ + execl(fp, "-y", "-i", r_fn, "-map_metadata", "-1", "-c", "copy", tmp_fn, (char*) NULL); + _exit(1); + } + + if (waitpid(pid, &ret, 0) == -1) { + srv_log(xs_fmt("strip_media: cannot waitpid()")); + return -1; + } if (ret != 0) { - int code = 0; - if (WIFEXITED(ret)) - code = WEXITSTATUS(ret); - - if (code == 127) - srv_log(xs_fmt("strip_media: error stripping %s. '%s' not found (exit 127). Set 'ffmpeg_path' in server.json.", r_fn, fp)); - else { - srv_log(xs_fmt("strip_media: error stripping %s %d", r_fn, ret)); - srv_log(xs_fmt("strip_media: command was: %s", cmd)); - } - + srv_log(xs_fmt("strip_media: error stripping %s %d", r_fn, ret)); + /* try to cleanup, just in case */ /* unlink needs full path too if we are not in basedir */ xs *full_tmp_fn = xs_fmt("%s/%s", srv_basedir, tmp_fn); @@ -286,38 +327,37 @@ int strip_media(const char *fn) int check_strip_tool(void) +/* check if strip_exif tools do exist and fix their absolute path */ { const xs_val *v = xs_dict_get(srv_config, "strip_exif"); + /* skip if unless strip_exif; return non-error */ + if (xs_type(v) != XSTYPE_TRUE) + return 1; + int ret = 1; + const char *progs[] = { "ffmpeg", "mogrify" }; - if (xs_type(v) == XSTYPE_TRUE) { - /* check mogrify */ - { - const char *mp = xs_dict_get(srv_config, "mogrify_path"); - if (mp == NULL) - mp = "mogrify"; - - xs *cmd = xs_fmt("%s -version 2>/dev/null >/dev/null", mp); - - if (system(cmd) != 0) { - srv_log(xs_fmt("check_strip_tool: '%s' not working", mp)); - ret = 0; - } + for (int i = 0; i < (int)(sizeof(progs) / sizeof(progs[0])); i++) { + xs_str *key = xs_fmt("%s_path", progs[i]); + + const char *val = xs_dict_get(srv_config, key); + if (val == NULL) { + val = findprog(progs[i]); + if (val != NULL) + srv_debug(1, xs_fmt("check_strip_tool: found %s in PATH at %s", progs[i], val)); } - /* check ffmpeg */ - if (ret) { - const char *fp = xs_dict_get(srv_config, "ffmpeg_path"); - if (fp == NULL) - fp = "ffmpeg"; - - xs *cmd = xs_fmt("%s -version 2>/dev/null >/dev/null", fp); - - if (system(cmd) != 0) { - srv_log(xs_fmt("check_strip_tool: '%s' not working", fp)); - ret = 0; - } + if (val == NULL) { + srv_log(xs_fmt("check_strip_tool: %s not found in PATH", progs[i])); + ret = 0; + } else if (access(val, X_OK) != 0) { + srv_log(xs_fmt("check_strip_tool: %s '%s' is not executable", progs[i], val)); + ret = 0; + } else { + srv_config = xs_dict_set(srv_config, key, val); } + + xs_free(key); } return ret; diff --git a/snac.h b/snac.h index dfc12cb..dfed18a 100644 --- a/snac.h +++ b/snac.h @@ -107,6 +107,7 @@ int validate_uid(const char *uid); xs_str *hash_password(const char *uid, const char *passwd, const char *nonce); int check_password(const char *uid, const char *passwd, const char *hash); +char* findprog(const char *prog); int strip_media(const char *fn); int check_strip_tool(void); -- cgit v1.2.3