From fe94692778d741b0c71946763a3c0d1ebf3e59e5 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 22 May 2026 15:26:47 +0000 Subject: [PATCH 1/3] Fix buffer overflow in patch-cmdline.c Replace `strcpy` with `strncpy` to prevent buffer overflows when copying the command line into the mapped buffer. Also fixed the `munmap` cleanup logic which was using an incorrect size. Co-authored-by: manupawickramasinghe <73810867+manupawickramasinghe@users.noreply.github.com> --- .jules/sentinel.md | 4 ++++ tools/patch-image/src/patch-cmdline.c | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index fcf926b7d82870..ec7435eb445417 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -102,3 +102,7 @@ **Vulnerability:** In `package/network/config/netifd/files/lib/netifd/utils.uc`, the `handler_load` function iterates over `.sh` scripts in a directory and uses their `basename` to execute them via a string interpolated `system()` call (`system("./${script} ...")`). If an attacker could place a maliciously named file in the parsed directory (e.g., `$(touch \/tmp\/pwned).sh`), it would result in arbitrary command execution. **Learning:** In `ucode` scripts, `system()` with a string argument is executed by the shell (`/bin/sh -c`). When using variables derived from filenames or external sources within these string templates, failure to sanitize allows shell metacharacter injection. **Prevention:** To protect `system()` or `fs.popen()` when string interpolation is unavoidable, always strictly validate variables using regex allowlists (e.g., `if (match(script, /[^a-zA-Z0-9_.-]/)) continue;`) to ensure only safe characters are evaluated by the shell. +## 2024-05-24 - Buffer overflow and munmap leak in patch-cmdline.c +**Vulnerability:** A `strcpy` copied `argv[2]` into a buffer bounded by `CMDLINE_MAX` leading to buffer overflow, and an error path `munmap` passed the wrong length (`len` instead of `search_space + CMDLINE_MAX`). +**Learning:** Even when there is length checking logic (`len + 9 > CMDLINE_MAX`), `strcpy` should not be used as it does not inherently limit the bounds of the destination buffer based on size limit. Also, mmap cleanup sizes must match the original mmap size to avoid leaks and memory corruption. +**Prevention:** Always use bounded copy functions like `strncpy` or `strscpy`, making sure the buffer size includes room for null termination, and carefully double-check `munmap` sizes against their matching `mmap` allocation. diff --git a/tools/patch-image/src/patch-cmdline.c b/tools/patch-image/src/patch-cmdline.c index 7a54f81b72180b..13967c5791eb60 100644 --- a/tools/patch-image/src/patch-cmdline.c +++ b/tools/patch-image/src/patch-cmdline.c @@ -71,12 +71,12 @@ int main(int argc, char **argv) } memset(p, 0, CMDLINE_MAX - 8); - strcpy(p, argv[2]); + strncpy(p, argv[2], CMDLINE_MAX - 9); msync(p, CMDLINE_MAX, MS_SYNC|MS_INVALIDATE); ret = 0; err3: - munmap((void *) ptr, len); + munmap((void *) ptr, search_space + CMDLINE_MAX); err2: if (fd > 0) close(fd); From 001dcb90a51869a7e71dc8bfdf7195ce9fbf926d Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 22 May 2026 15:30:45 +0000 Subject: [PATCH 2/3] tools/patch-image: Fix buffer overflow in patch-cmdline.c Replace `strcpy` with `strncpy` to prevent buffer overflows when copying the command line into the mapped buffer. Also fixed the `munmap` cleanup logic which was using an incorrect size. Signed-off-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: manupawickramasinghe <73810867+manupawickramasinghe@users.noreply.github.com> From e8f6e0b7e492b593f714f7ad769fed5dba7ca6cb Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 22 May 2026 15:39:17 +0000 Subject: [PATCH 3/3] tools/patch-image: Fix buffer overflow in patch-cmdline.c Replace `strcpy` with `strncpy` to prevent buffer overflows when copying the command line into the mapped buffer. Also fixed the `munmap` cleanup logic which was using an incorrect size. Signed-off-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: manupawickramasinghe <73810867+manupawickramasinghe@users.noreply.github.com>