From a0c06d0843a7dd8c8e2603913cd0d33c1efa78ea Mon Sep 17 00:00:00 2001 From: NRK Date: Sun, 5 Jan 2025 19:17:18 +0000 Subject: [PATCH 1/5] xstrsncpy: allow 0 len copies to be no-ops --- src/nnn.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/nnn.c b/src/nnn.c index 5c7e136f..81af4c5b 100644 --- a/src/nnn.c +++ b/src/nnn.c @@ -988,7 +988,8 @@ static size_t xstrsncpy(char *restrict dst, const char *restrict src, size_t n) char *end = memccpy(dst, src, '\0', n); if (!end) { - dst[n - 1] = '\0'; // NOLINT + if (n) + dst[n - 1] = '\0'; end = dst + n; /* If we return n here, binary size increases due to auto-inlining */ } From 1cf9654368c79b013b8b77f26c89f2e078dacc51 Mon Sep 17 00:00:00 2001 From: NRK Date: Wed, 22 Nov 2023 23:55:28 +0600 Subject: [PATCH 2/5] remove malloc casting --- src/nnn.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/nnn.c b/src/nnn.c index 81af4c5b..e19b7afe 100644 --- a/src/nnn.c +++ b/src/nnn.c @@ -2332,7 +2332,7 @@ static char *parseargs(char *cmd, char **argv, int *pindex) { int count = 0; size_t len = xstrlen(cmd) + 1; - char *line = (char *)malloc(len); + char *line = malloc(len); if (!line) { DPRINTF_S("malloc()!"); @@ -8562,8 +8562,8 @@ static bool setup_config(void) if (!xdg) len = xstrlen(home) + xstrlen("/.config/nnn/bookmarks") + 1; - cfgpath = (char *)malloc(len); - plgpath = (char *)malloc(len); + cfgpath = malloc(len); + plgpath = malloc(len); if (!cfgpath || !plgpath) { xerror(); return FALSE; @@ -8601,7 +8601,7 @@ static bool setup_config(void) char *env_sel = xgetenv(env_cfg[NNN_SEL], NULL); selpath = env_sel ? xstrdup(env_sel) - : (char *)malloc(len + 3); /* Length of "/.config/nnn/.selection" */ + : malloc(len + 3); /* Length of "/.config/nnn/.selection" */ if (!selpath) { xerror(); From 94bac4ef986aed99cfc88963d6a429dce2c89058 Mon Sep 17 00:00:00 2001 From: NRK Date: Sun, 5 Jan 2025 19:40:33 +0000 Subject: [PATCH 3/5] shell_escape: return strlen or -1 on failure also check for `outlen < 3` since the function unconditionally writes at least 3 bytes. --- src/nnn.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/nnn.c b/src/nnn.c index e19b7afe..936b19f9 100644 --- a/src/nnn.c +++ b/src/nnn.c @@ -1335,21 +1335,23 @@ static char *bmtarget(const char *filepath, char *cwd, char *buf) } /* wraps the argument in single quotes so it can be safely fed to shell */ -static bool shell_escape(char *output, size_t outlen, const char *s) +static ssize_t shell_escape(char *output, size_t outlen, const char *s) { size_t n = xstrlen(s), w = 0; - if (s == output) { - DPRINTF_S("s == output"); - return FALSE; + if (s == output || outlen < 3) { + errno = EINVAL; + return -1; } output[w++] = '\''; /* begin single quote */ for (size_t r = 0; r < n; ++r) { /* potentially too big: 4 for the single quote case, 2 from * outside the loop */ - if (w + 6 >= outlen) - return FALSE; + if (w + 6 >= outlen) { + errno = ENAMETOOLONG; + return -1; + } switch (s[r]) { /* the only thing that has special meaning inside single @@ -1366,8 +1368,8 @@ static bool shell_escape(char *output, size_t outlen, const char *s) } } output[w++] = '\''; /* end single quote */ - output[w++] = '\0'; /* nul terminator */ - return TRUE; + output[w] = '\0'; /* nul terminator */ + return w; } static bool set_tilde_in_path(char *path) @@ -2841,7 +2843,7 @@ static void archive_selection(const char *cmd, const char *archive) static void write_lastdir(const char *curpath, const char *outfile) { - bool tilde = false; + bool tilde = false, ok = false; if (!outfile) xstrsncpy(cfgpath + xstrlen(cfgpath), "/.lastd", 8); else @@ -2850,15 +2852,14 @@ static void write_lastdir(const char *curpath, const char *outfile) int fd = open(outfile ? (tilde ? g_buf : outfile) : cfgpath, O_CREAT | O_WRONLY | O_TRUNC, S_IWUSR | S_IRUSR); - - if (fd != -1 && shell_escape(g_buf, sizeof(g_buf), curpath)) { - if (write(fd, "cd ", 3) == 3) { - if (write(fd, g_buf, strlen(g_buf)) != (ssize_t)strlen(g_buf)) { - DPRINTF_S("write failed!"); - } - } + if (fd >= 0) { + memcpy(g_buf, "cd ", 3); // NOLINT + ssize_t l = shell_escape(g_buf + 3, sizeof(g_buf) - 3, curpath); + ok = l >= 0 && write(fd, g_buf, l + 3) == l + 3; close(fd); } + if (!ok) + errexit(); } /* From 9d33ba1104b390256f72f8203212720b95447b38 Mon Sep 17 00:00:00 2001 From: NRK Date: Sun, 5 Jan 2025 21:55:06 +0000 Subject: [PATCH 4/5] icons-hash: table needs to be strictly bigger than icons --- src/icons-hash.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/icons-hash.c b/src/icons-hash.c index 699af33b..5ad4a0f4 100644 --- a/src/icons-hash.c +++ b/src/icons-hash.c @@ -10,7 +10,7 @@ #define GOLDEN_RATIO_32 UINT32_C(2654442313) /* golden ratio for 32bits: (2^32) / 1.61803 */ #define GOLDEN_RATIO_64 UINT64_C(0x9E3793492EEDC3F7) -#define ICONS_TABLE_SIZE 8 /* size in bits. 8 = 256 */ +#define ICONS_TABLE_SIZE 8 /* size in bits. 8 = 2^8 = 256 */ #ifndef TOUPPER #define TOUPPER(ch) (((ch) >= 'a' && (ch) <= 'z') ? ((ch) - 'a' + 'A') : (ch)) @@ -118,7 +118,7 @@ pcg(uint64_t *state) int main(void) { - ENSURE(ARRLEN(icons_ext) <= ARRLEN(table)); + ENSURE(ARRLEN(icons_ext) < ARRLEN(table)); ENSURE(ICONS_TABLE_SIZE < 16); ENSURE(1u << ICONS_TABLE_SIZE == ARRLEN(table)); ENSURE((GOLDEN_RATIO_32 & 1) == 1); /* must be odd */ From 6a9bf206cfe09ef7d9121cd6c87b345d44d48a43 Mon Sep 17 00:00:00 2001 From: NRK Date: Sun, 5 Jan 2025 22:20:00 +0000 Subject: [PATCH 5/5] don't feed unchecked fd to dup2() the open() might fail and then dup2 will be called with -1 fd. clang-tidy complains about this. just open "/dev/null" once at program startup, check the result, and then reuse that fd throughout. --- src/nnn.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/nnn.c b/src/nnn.c index 936b19f9..ddedd78e 100644 --- a/src/nnn.c +++ b/src/nnn.c @@ -438,6 +438,7 @@ static int nselected; #ifndef NOFIFO static int fifofd = -1; #endif +static int devnullfd = -1; static time_t gtimesecs; static uint_t idletimeout, selbufpos, selbuflen; static ushort_t xlines, xcols; @@ -2487,13 +2488,10 @@ static int spawn(char *file, char *arg1, char *arg2, char *arg3, ushort_t flag) if (pid == 0) { /* Suppress stdout and stderr */ if (flag & F_NOTRACE) { - int fd = open("/dev/null", O_WRONLY, 0200); - if (flag & F_NOSTDIN) - dup2(fd, STDIN_FILENO); - dup2(fd, STDOUT_FILENO); // NOLINT - dup2(fd, STDERR_FILENO); - close(fd); + dup2(devnullfd, STDIN_FILENO); + dup2(devnullfd, STDOUT_FILENO); + dup2(devnullfd, STDERR_FILENO); } else if (flag & F_TTY) { /* If stdout has been redirected to a non-tty, force output to tty */ if (!isatty(STDOUT_FILENO)) { @@ -8859,6 +8857,12 @@ int main(int argc, char *argv[]) DPRINTF_S(VERSION); #endif + devnullfd = open("/dev/null", O_RDWR | O_CLOEXEC); + if (devnullfd < 0) { + xerror(); + return EXIT_FAILURE; + } + /* Prefix for temporary files */ if (!set_tmp_path()) return EXIT_FAILURE;