From 9485c7867156c2a02139edcf77156b526879dd29 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sun, 14 Jun 2026 01:59:52 +0000 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[MEDIUM?= =?UTF-8?q?]=20Fix=20command=20injection=20vulnerability=20in=20unetd?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Properly escape shell variables `out_file` and `bin_path` within `package/network/services/unetd/files/unet.uc` by replacing any internal single quotes and wrapping them in single quotes to prevent potential command injection when passing arguments to `sh -c`. Co-authored-by: manupawickramasinghe <73810867+manupawickramasinghe@users.noreply.github.com> --- .jules/sentinel.md | 133 +------------------ package/network/services/unetd/files/unet.uc | 6 +- 2 files changed, 8 insertions(+), 131 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index f419076973b5fc..7e2f76bce1378c 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -1,129 +1,4 @@ -## 2024-03-13 - [Fix SSL Certificate Verification in GitHub Archive Downloader] -**Vulnerability:** `scripts/dl_github_archive.py` created an unverified SSL context (`ssl._create_unverified_context()`) when making requests to the GitHub API, disabling SSL/TLS certificate verification. This left the script vulnerable to Man-in-the-Middle (MITM) attacks. -**Learning:** This vulnerability existed likely to bypass SSL errors on developer machines with outdated or missing CA certificates. -**Prevention:** Always use `ssl.create_default_context()` or let the system use its default verified context when making HTTPS requests. Do not disable SSL verification in production scripts, especially when downloading source code archives. -## 2025-05-18 - [CRITICAL] Fix command injection in unet.uc -**Vulnerability:** The network name parameter was only checked for forward slashes before being directly substituted into shell strings, leading to command injection and path traversal risks via `system()` calls. -**Learning:** `ucode` scripts using `system()` with backticks or string concatenation are highly vulnerable to injection if variables are not rigorously validated, especially in user-facing configuration paths. -**Prevention:** Always use regex matching (e.g., `match(name, /[^a-zA-Z0-9_-]/)`) to strictly whitelist input characters for system parameters before interpolating them into shell commands. -## 2024-05-23 - [CRITICAL] Fix command injection in unet.uc via network_keygen config -**Vulnerability:** The `network_keygen` function in `unet.uc` constructed shell command strings without validating configuration variables like `config.rounds` and `config.salt`, leading to a command injection vulnerability. An attacker capable of modifying config JSON files could inject arbitrary commands. -**Learning:** Even internal configuration objects like `config.salt` must be strictly validated before being interpolated into `system()` strings to prevent command injection, especially when restoring configurations from JSON. -**Prevention:** Use strict regex whitelists (e.g., `match(salt, /[^a-fA-F0-9]/)`) to validate integer and hex inputs before using them in shell commands. -## 2025-05-18 - [CRITICAL] Fix command injection in ucode network scripts -**Vulnerability:** Several ucode scripts (`unet.uc`, `mac80211.sh`, `wdev.uc`, `packet-steering.uc`) used string interpolation to construct shell commands passed to `system()`. If configuration parameters like `config.txpower`, `config.country`, `config.distance`, or `wdev.freq` were malicious, an attacker could execute arbitrary code by injecting shell metacharacters. -**Learning:** `system()` in ucode acts like `os.system` when passed a string, invoking the shell interpreter (`sh -c`). However, when passed an array (e.g., `system(["iw", "phy", phy, "set", "txpower", config.txpower])`), it acts like `execvp`, directly executing the binary without shell parsing. This completely eliminates command injection risks without the need for complex, manual regex validation. -**Prevention:** Always use the array-based syntax for `system()` calls in ucode scripts to safely pass variables as discrete arguments. Only use string-based `system()` when explicit shell features like redirection (`>`) are required, and carefully validate or quote variables in those specific cases (or wrap them explicitly in `system(["sh", "-c", "..."])` for clarity). -## 2024-05-24 - [CRITICAL] Fix buffer overflow in ead client -**Vulnerability:** ead-client.c and t_pw.c used strcpy to copy user input into fixed-size character arrays without checking the length, causing buffer overflows. -**Learning:** Legacy string handling mechanisms that map structures onto network packets or directly copy from command line arguments are vulnerable when using unsafe functions like strcpy. -**Prevention:** Always use safe string operations like strncpy, and ensure that buffers are correctly bounded and null-terminated. -## 2025-02-18 - Insecure Temporary File Creation in t_misc.c -**Vulnerability:** Predictable temporary file creation using `sprintf(dotpath, "/tmp/rnd.%d", getpid())` followed by `creat()` in `t_fshash()`. -**Learning:** This classic pattern (CWE-377) allows local attackers to pre-create symlinks at predictable paths, potentially causing arbitrary file overwrite or manipulation. `getpid()` is easily predictable. -**Prevention:** Always use `mkstemp()` to securely and atomically create temporary files with random names and safe permissions (O_EXCL). Use `fstat(fd, ...)` on the resulting file descriptor instead of `stat(path, ...)` to avoid TOCTOU race conditions. -## 2024-05-24 - Buffer Overflow in EAD Client Username -**Vulnerability:** The `ead-client` uses `strcpy` to copy a user-provided command-line argument (`username`) into a fixed-size buffer (`char username[32]`) in the `ead_msg_user` struct, leading to a stack-smashing buffer overflow if the username exceeds 31 characters. -**Learning:** Older C components might still rely on unbounded string operations for data that comes from local process arguments, trusting local input. -**Prevention:** Always use `strncpy` or `strlcpy` and explicitly null-terminate the buffer when copying arbitrary user input (even local CLI arguments) into fixed-size message structs. -## 2024-05-24 - Out-of-bounds write in ead.c message decryption -**Vulnerability:** In `package/network/services/ead/src/ead.c`, `handle_send_cmd` decrypts a message from the network and subtracts a header size to get `datalen`. Without an upper bound check, `datalen` could exceed the fixed buffer size (`msgbuf`), allowing `cmd->data[datalen] = 0;` to write a null byte out-of-bounds, potentially causing a crash or remote code execution. -**Learning:** Decrypted network payloads must always have their length validated against an upper limit before being used as an index, especially when writing to static buffers in C network services. -**Prevention:** Always add explicit upper bounds checks on decrypted data lengths (e.g., `if (datalen > 1024) return false;`) immediately after lower bounds checks. -## 2024-05-24 - Buffer Overflow in ead-client cmd Argument -**Vulnerability:** A heap/BSS buffer overflow existed in `package/network/services/ead/src/ead-client.c`'s `send_command` due to unchecked `strlen(command)` when calculating the message size for `ead_encrypt_message`. Though `strncpy` capped copying to 1024 bytes, a lack of null-termination and passing `strlen(command)` to encryption allowed out-of-bounds reads/writes if the command string exceeded 1024 bytes. -**Learning:** Even when bounded string copying functions (`strncpy`) are used, if the original unbounded length (`strlen`) is reused for subsequent buffer sizing or cryptographic operations on fixed-size buffers, it can lead to memory corruption out-of-bounds of the allocated structure (`struct ead_msg` / `msgbuf`). -**Prevention:** Always calculate and store the bounded length into a local variable *first*, then strictly use that bounded length for all subsequent buffer allocations, string copies (ensuring explicit null-termination), and size calculations within the function block. -## 2025-03-21 - Stack Buffer Overflow in PID file Creation -**Vulnerability:** The `ead` service writes its process ID to a file using `sprintf(pid, "%d\n", getpid())` into a stack-allocated buffer `char pid[8];`. Since `getpid()` can return up to 7 digits on modern Linux systems, adding `\n` and `\0` requires at least 9 bytes, overflowing the 8-byte buffer. -**Learning:** Hardcoded small buffer sizes for formatting integer PIDs can lead to stack memory corruption (CWE-121) as OS configurations like `kernel.pid_max` allow PIDs to exceed traditional limits. -**Prevention:** Always allocate sufficiently large stack buffers (e.g., 32 bytes) when formatting PIDs, or preferably use `snprintf` with `sizeof(buffer)` to enforce explicit bounds checking. -## 2024-05-30 - [ead: fix buffer caps and string null-terminations] -**Vulnerability:** Off-by-one out-of-bounds null-byte poisoning write and missing strict null-terminations when using `strncpy` in C network components (`ead`). -**Learning:** `datalen` checking allowed `datalen` to be equal to the max buffer size constraint (`1024`) leading to out-of-bounds index writing (`cmd->data[1024] = 0;` on a 1024-byte buffer). Additionally, using `strncpy` to write into an uninitialized bounded buffer without a subsequent explicit null-termination can result in a non-terminated string if the source length equals the buffer length. -**Prevention:** Always bounds-check network payloads strictly against constraints using `>=` when determining lengths for null-termination array indexes. Additionally, always manually null-terminate strings copied with `strncpy` explicitly via `buf[sizeof(buf) - 1] = '\0'`. - -## 2024-05-30 - [github actions: migrate toolchain archives from .tar.xz to .tar.zst] -**Learning:** OpenWrt has migrated toolchain snapshots to `.tar.zst` format instead of `.tar.xz`, which caused the Github Actions CI to fail with `404 Not Found` when trying to download `openwrt-toolchain-*.tar.xz` or `openwrt-sdk-*.tar.xz`. -**Action:** Always check the `.tar.zst` archive format for toolchains/SDKs in `build.yml` when fetching from `downloads.cdn.openwrt.org`. Ensure that `tar --zstd -xf -` is used in place of `tar --xz -xf -`. -## 2024-05-30 - [github actions: install missing zstd package for toolchains] -**Learning:** After OpenWrt's migration from `.tar.xz` to `.tar.zst` for toolchain archives, the `ghcr.io/zektopic/tools:latest` container lacks the `zstd` binary, causing tar extractions to fail (`tar (grandchild): zstd: Cannot exec: No such file or directory`). -**Action:** When migrating tar archives from `.xz` to `.zst`, ensure the CI container environment is capable of unpacking them by installing `zstd` via `apt-get install -y zstd` in the environment initialization step. -## 2024-11-20 - Fix buffer overflow risk in t_pw.c -**Vulnerability:** Unbounded `strcpy` operations copying external usernames into fixed-size struct buffers (`userbuf`, `pebuf.name`) of size `MAXUSERLEN` in `ead/src/tinysrp/t_pw.c`. -**Learning:** Legacy C code in the `tinysrp` dependency relied on upstream parser constraints (`t_nextfield`) for bounds-checking, exposing the credential structures to stack smashing if parser limits changed or failed to explicitly null-terminate strings. -**Prevention:** Use explicitly bounded `strncpy(dest, src, MAXUSERLEN - 1)` with manual null-termination (`dest[MAXUSERLEN - 1] = '\0'`) when persisting credential strings within fixed structs. -## 2024-05-24 - Buffer Overflow in ead.c Bridge Assignment -**Vulnerability:** The `check_bridge_port` function in `ead.c` copies a bridge name into a fixed-size 16-byte buffer (`in->bridge`) using `strncpy(in->bridge, br, sizeof(in->bridge));` without explicit null termination. If the bridge name `br` is 16 bytes or longer, it won't be null-terminated, which leads to out-of-bounds reads in the subsequent `DEBUG` log and in a call to `ead_open_pcap`. -**Learning:** `strncpy` does not guarantee null-termination if the source string length is greater than or equal to the buffer size. This classic pitfall can lead to out-of-bounds reads when the buffer is later used as a C string. -**Prevention:** Always explicitly null-terminate the destination buffer after calling `strncpy`, or use safer alternatives like `strlcpy` or `snprintf` when available. -## 2026-04-13 - [CRITICAL] Fix undefined behavior and buffer termination in EAD -**Vulnerability:** A `static const char password[MAXPARAMLEN]` buffer was being cast to `char *` and modified directly via `strncpy`, resulting in undefined behavior. Additionally, multiple `strncpy` calls in the `ead` and `ead-client` code failed to correctly ensure explicit null-termination if the source string exactly matched or exceeded the destination buffer length. -**Learning:** Legacy C network services frequently mix `strncpy` with incorrect length bounds or forget manual null-termination, leading to out-of-bounds reads when subsequent string operations process the buffer. Furthermore, using `const` arrays for mutable global state via pointer casting is a dangerous pattern that can lead to subtle bugs or compiler-optimized crashes. -**Prevention:** Always declare buffers intended for mutation without the `const` qualifier. When using `strncpy`, ensure the maximum length copied is `buffer_size - 1` and explicitly set the final byte to `\0` (`buffer[buffer_size - 1] = '\0'`) to guarantee safety across all string operations. - -## 2024-05-18 - Missing Buffer Overflows Bounds in Network Parsing -**Vulnerability:** Bounds checks were missing on values read directly from network payloads in the Emergency Access Daemon service (ead & ead-client), specifically regarding string boundaries and payload length integer checking. -**Learning:** `ead-client.c` lacked bounds checking on network message values resulting in possible Buffer Overflow scenarios such as when `handle_pong` null terminates an unbounded network string, or when `memcpy` calls assume the length is within allocation boundaries. An integer wrap-around could also be induced by negative payloads passing unsigned comparisons. -**Prevention:** Always bound dynamically decoded/deserialized values from network packets (e.g. `ntohl()`) and assert that they are within memory constraints prior to executing unsafe string manipulation functions or memory copy operations. Use minimum bound checks, ensure sizes correctly restrict payload bounds to allocated structures, and clamp buffer pointers to structure max allocation lengths. -## 2024-05-09 - [ead-client.c buffer overflow vulnerabilities] -**Vulnerability:** Multiple buffer overflows in the OpenWrt Emergency Access Daemon client (`ead-client.c`) due to missing bounds checks on variables `len` (derived from attacker-controlled `msg->len`) before using them as index assignments (`pong->name[len] = 0;`) or size limits in `memcpy` operations. -**Learning:** Decrypting untrusted network data (`ead_decrypt_message()`) does not guarantee the contents or length of the data are valid for local struct allocations. Values computed from network headers (like `msg->len`) must be strictly validated against constant bounds before being used in memory operations. -**Prevention:** Always implement explicit maximum bounds checks on derived length variables before using them in functions like `memcpy` or array assignments, particularly when the length affects fixed-size buffers. Also ensure lengths don't evaluate to <= 0 before doing arithmetic or using them as indices. -## 2025-04-03 - Buffer Overflow in tinysrp Usernames -**Vulnerability:** The `tinysrp` library in `ead` used legacy `strcpy` to copy credentials like usernames into fixed-size buffers (`tpw->userbuf`), leading to a potential stack-smashing buffer overflow if the username exceeds `MAXUSERLEN`. -**Learning:** Legacy C code often relies on unbounded string operations. Usernames, being external input, should not be trusted to fit within statically allocated memory limits like `MAXUSERLEN`. -**Prevention:** Always replace legacy `strcpy` calls with `strncpy` bounded by the target buffer size minus one (e.g., `MAXUSERLEN - 1`) and append explicit null-termination when handling credentials to prevent buffer overflow vulnerabilities. - -## 2026-04-13 - Buffer Overflow Prevention in t_misc.c -**Vulnerability:** In `package/network/services/ead/src/tinysrp/t_misc.c`, the `t_fshash` function utilized unbounded `strcpy` and `strcat` calls into a fixed size 128 byte buffer `dotpath` in a loop modifying paths. If the loop bounds were altered or an unpredictable temporary file suffix increased the length, this could lead to stack buffer overflows. -**Learning:** Legacy string manipulation functions (`strcpy`, `strcat`) easily introduce memory corruption bugs and are vulnerable to future modifications changing implicit bounds. -**Prevention:** Always replace `strcat` and `strcpy` with explicitly bounded calls such as `strncpy` and `strncat`, and calculate remaining capacities properly (e.g. `sizeof(buf) - strlen(buf) - 1`). Explicit null-termination `buf[sizeof(buf) - 1] = '\0'` should also be added for safety. - -## 2025-05-24 - Buffer Overflow Prevention in trelay.c -**Vulnerability:** In `package/kernel/trelay/src/trelay.c`, the function `trelay_do_add` used unbounded `strcpy` to copy strings to a dynamically allocated structure using a flexible array member. While the allocation size included `strlen(name) + 1`, a discrepancy between evaluation and usage could theoretically lead to overflows or TOCTOU issues. -**Learning:** Separate calculation of bounds when dealing with variable length allocations combined with unbounded string routines can be fragile. -**Prevention:** Always use safe bounded string copy mechanisms like `strscpy`, use variables to reuse the calculated `strlen` for the memory allocation size, and rely on this same value to constrain the subsequent copy operations to guarantee safety. - -## 2026-04-13 - Prevent TOCTOU and Buffer Overflow in trelay -**Vulnerability:** In `package/kernel/trelay/src/trelay.c`, the `trelay_do_add` function previously used `strlen(name) + 1` directly within `struct_size` for allocating memory with `kzalloc`, and later used an unbounded `strcpy` to copy the `name` string into the structure's flexible array member. If the `name` string was dynamically altered between the memory allocation size calculation and the string copy (Time-of-Check to Time-of-Use), it could result in an unbounded `strcpy` causing an out-of-bounds write buffer overflow. -**Learning:** In Linux kernel modules (e.g., the `trelay` package), using `strcpy` is unsafe. Dynamically calculating length multiple times for flexible array member allocations and subsequent copies can lead to TOCTOU vulnerabilities if the source buffer contents change between checks. -**Prevention:** Avoid unsafe `strcpy`. Use bounded functions like `strscpy`. When copying into dynamically allocated structures with flexible array members, calculate the required length (including the null terminator) once, store it in a variable (e.g., `size_t name_len`), and use that same variable for both the memory allocation (via `struct_size`) and the bounded copy operation (`strscpy`) to ensure consistency and prevent TOCTOU vulnerabilities. -## 2024-05-24 - Buffer Overflow Prevention in nvram.c -**Vulnerability:** In `package/utils/nvram/src/nvram.c`, the `_nvram_realloc` function used unbounded `strcpy` to copy strings into dynamically allocated structures. While the allocation sizes were based on `strlen(name) + 1` and `strlen(value) + 1`, a discrepancy between evaluation and usage, or potential integer overflows during size calculation, could theoretically lead to heap buffer overflows. Furthermore, casting pointers to `int` for alignment checks (`(int)ptr % 4`) caused compiler warnings on 64-bit platforms and could lead to incorrect alignment logic if the pointer value exceeded the bounds of an `int`. -**Learning:** Legacy string manipulation functions (`strcpy`) are unsafe and easily introduce memory corruption bugs. Unchecked arithmetic on string lengths before allocation can lead to integer overflows, resulting in under-sized buffers. Casting pointers to `int` is non-portable and unsafe on 64-bit architectures. -**Prevention:** Always use safe bounded string copy mechanisms like `strlcpy`. Implement explicit `SIZE_MAX` overflow checks when calculating allocation sizes, especially for user-controlled or external input. Use variables to cache the calculated `strlen` for both memory allocation size and subsequent copy operations to guarantee safety and consistency. For pointer arithmetic or alignment checks, use `uintptr_t` to ensure 64-bit compatibility. -## 2024-05-01 - Prevent Command Injection in ucode fs.popen -**Vulnerability:** Command injection via unvalidated variables passed to `fs.popen()` using template literals. -**Learning:** In OpenWrt `ucode` scripts, `fs.popen()` strictly requires a string argument and does not support arrays, meaning shell metacharacters in interpolated variables are evaluated by the shell. -**Prevention:** Strictly validate any external or dynamic variables used in `fs.popen()` strings with regex allowlists (e.g., `match(var + "", /[^a-zA-Z0-9_.-]/)`) before execution. - -## 2024-05-27 - Command Injection in netifd utils -**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. -## 2024-05-24 - Buffer Overflow via `sprintf` in tinysrp/tphrase.c -**Vulnerability:** A buffer overflow vulnerability existed because `sprintf` was used to write to dynamically allocated buffers (`bakfile` and `bakfile2`) without bounds checking. While the buffer size was mathematically exact (`strlen(pwname) + 5`), `sprintf` is inherently unsafe and flagged by security scanners. -**Learning:** `sprintf` lacks bounds checking, making it an anti-pattern even when buffer sizes are "known" to be correct, as subsequent changes to the formatting string or length calculation could easily introduce overflows. -**Prevention:** Always use bounded string formatting functions like `snprintf` instead of `sprintf`. Calculate and reuse the allocation size variable for both the buffer allocation and the `snprintf` bounds check to ensure consistency and prevent off-by-one errors. -## 2026-05-22 - [Fix buffer overflow risk in AR8327 LED driver] -**Vulnerability:** Unbounded `strcpy` used for copying dynamic string into flexible array member without reusing precalculated length. -**Learning:** In kernel modules, `strcpy` should be replaced with `strscpy`, and size variables should be precalculated to avoid TOCTOU races between allocation and copying. -**Prevention:** Use `strscpy` with a precalculated length variable instead of `strcpy` and `strlen`. -## 2025-02-28 - Command Injection in unet.uc via sh -c interpolation -**Vulnerability:** In `package/network/services/unetd/files/unet.uc`, the `network_keygen` function executed a shell command (`sh -c`) using string concatenation where a variable (`extra_args`) was wrapped in double quotes but not properly escaped, allowing potential command injection. -**Learning:** String interpolation for shell execution in `ucode` using `sh -c` introduces command injection risks, even when wrapped in double quotes, because double quotes do not prevent the shell from evaluating variables (`$`) or subshells (`` ` ``). -**Prevention:** Avoid `sh -c` and use array-based arguments for `system()` or `fs.popen()` when possible. When `sh -c` is unavoidable (e.g., for IO redirection), strictly escape dynamic variables by replacing single quotes (e.g., `replace(var, /'/g, "'\\''")`) and wrapping the result in single quotes (`'...'`). - -## 2024-05-25 - Prevent Command Injection via fs.popen() -**Vulnerability:** A shell command injection vulnerability existed where attacker-controllable variables (e.g. `num_global_macaddr`, `mbssid`) read from potentially unvalidated configuration sources were interpolated into a shell string executed by `fs.popen()`. -**Learning:** `fs.popen()` executes a shell implicitly and requires strict validation of any interpolated variables. In `ucode`, while `system()` can take an array, `fs.popen()` must take a string. -**Prevention:** Strictly validate any external input bound for `fs.popen()` interpolation using regex (e.g., `match(var + "", /[^0-9]/)`) to ensure malicious shell tokens are rejected. -## 2024-06-07 - Buffer Overflow Risk via sprintf in describe_vendor -**Vulnerability:** In `package/network/config/ltq-vdsl-vr9-app/src/src/dsl_cpe_ubus.c`, the `describe_vendor` function formatted a string using `sprintf(buf, "%s %d.%d", str, value[6], value[7])` into a fixed-size local 64-byte `buf`. If `str` were unexpectedly long, this would cause a stack-based buffer overflow. -**Learning:** `sprintf` lacks bounds checking and easily introduces memory corruption vulnerabilities when writing to fixed-size buffers, especially when input string lengths are not explicitly validated. -**Prevention:** Replace all usages of `sprintf` with explicitly bounded string formatting functions like `snprintf(buf, sizeof(buf), ...)`. This ensures the formatted string is safely truncated rather than overflowing the target buffer. +## 2025-06-14 - Fix Potential Command Injection in unetd +**Vulnerability:** Shell command injection in `unet.uc` where `bin_path` and `out_file` variables were directly interpolated into `system([ "sh", "-c", "..." ])` without escaping. +**Learning:** Even if input variables are validated via regex earlier in the code, injecting them unquoted or using single quotes without escaping them directly into `sh -c` introduces a vulnerability pattern if those validations are ever bypassed, refactored, or missed. +**Prevention:** Always escape shell variables using `replace(var, /'/g, "'\\''")` and enclose them in single quotes when interpolating into `sh -c`, or better yet, avoid `sh -c` entirely by using array-based execution when I/O redirection is not required. diff --git a/package/network/services/unetd/files/unet.uc b/package/network/services/unetd/files/unet.uc index 09c68fe1150bc0..8e73d742469fc4 100644 --- a/package/network/services/unetd/files/unet.uc +++ b/package/network/services/unetd/files/unet.uc @@ -70,7 +70,8 @@ function network_keygen(pw_file, args, config, out_file, extra_args) extra_args = "'" + replace(extra_args, /'/g, "'\\''") + "'"; else extra_args = ""; - cmd[2] += ` -s ${rounds},${salt} -o ${out_file}`; + let safe_out_file = "'" + replace(out_file, /'/g, "'\\''") + "'"; + cmd[2] += ` -s ${rounds},${salt} -o ${safe_out_file}`; if (config.xorkey) { xorkey = network_get_string_file(config.xorkey); @@ -817,7 +818,8 @@ function network_edit(ctx, argv) { let json_file = mkstemp(); let bin_path = "/etc/unetd/" + network + ".bin"; - if (system([ "sh", "-c", `unet-tool -T -b '${bin_path}' >&${json_file.fileno()}` ])) + let safe_bin_path = "'" + replace(bin_path, /'/g, "'\\''") + "'"; + if (system([ "sh", "-c", `unet-tool -T -b ${safe_bin_path} >&${json_file.fileno()}` ])) return; let json_data; From fd380bbf36d545f0a75750cc54180f2f4178b8f6 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sun, 14 Jun 2026 02:07:52 +0000 Subject: [PATCH 2/4] unetd: fix command injection vulnerability Properly escape shell variables `out_file` and `bin_path` within `package/network/services/unetd/files/unet.uc` by replacing any internal single quotes and wrapping them in single quotes to prevent potential command injection when passing arguments to `sh -c`. 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 39c5d73930a17644d25a55e65cd1d17119504a59 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sun, 14 Jun 2026 02:24:08 +0000 Subject: [PATCH 3/4] unetd: fix command injection vulnerability Properly escape shell variables `out_file` and `bin_path` within `package/network/services/unetd/files/unet.uc` by replacing any internal single quotes and wrapping them in single quotes to prevent potential command injection when passing arguments to `sh -c`. 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 583ee8142ffa66bbc1b63dcd58975460cceed31c Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sun, 14 Jun 2026 02:36:34 +0000 Subject: [PATCH 4/4] unetd: fix command injection vulnerability Properly escape shell variables `out_file` and `bin_path` within `package/network/services/unetd/files/unet.uc` by replacing any internal single quotes and wrapping them in single quotes to prevent potential command injection when passing arguments to `sh -c`. 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>