From d75cae80fa4ab7b5f6f39ddcbbbfbaa1a9183d81 Mon Sep 17 00:00:00 2001 From: Archit Gupta Date: Tue, 9 Jun 2026 00:55:01 -0700 Subject: [PATCH] makeBinaryWrapper: fix read past NUL When setting a prefix for a path-like environment variable, the deduplication code in set_env_prefix reads past the NUL byte at the end of the env val and into the next entry. This corrupts the resultant env value with data from the next env var, or other data sitting after it. --- .../makeBinaryWrapper/make-binary-wrapper.sh | 4 +- .../combination/combination.c | 4 +- pkgs/test/make-binary-wrapper/default.nix | 1 + .../overlength-strings/overlength-strings.c | 4 +- .../prefix-dedup-last/prefix-dedup-last.c | 64 +++++++++++++++++++ .../prefix-dedup-last.cmdline | 2 + .../prefix-dedup-last/prefix-dedup-last.env | 3 + pkgs/test/make-binary-wrapper/prefix/prefix.c | 4 +- 8 files changed, 78 insertions(+), 8 deletions(-) create mode 100644 pkgs/test/make-binary-wrapper/prefix-dedup-last/prefix-dedup-last.c create mode 100644 pkgs/test/make-binary-wrapper/prefix-dedup-last/prefix-dedup-last.cmdline create mode 100644 pkgs/test/make-binary-wrapper/prefix-dedup-last/prefix-dedup-last.env diff --git a/pkgs/by-name/ma/makeBinaryWrapper/make-binary-wrapper.sh b/pkgs/by-name/ma/makeBinaryWrapper/make-binary-wrapper.sh index 232bc2b5c3cd..595574468ffa 100644 --- a/pkgs/by-name/ma/makeBinaryWrapper/make-binary-wrapper.sh +++ b/pkgs/by-name/ma/makeBinaryWrapper/make-binary-wrapper.sh @@ -366,10 +366,10 @@ void set_env_prefix(char *env, char *sep, char *prefix) { return; } unsigned long sep_len = strlen(sep); - int n_before = existing_prefix - existing_env; + int n_before = existing_prefix - existing_env - sep_len; assert_success(asprintf(&val, \"%s%s%.*s%s\", prefix, sep, n_before, existing_env, - existing_prefix + prefix_len + sep_len)); + existing_prefix + prefix_len)); } else { assert_success(asprintf(&val, \"%s%s%s\", prefix, sep, existing_env)); } diff --git a/pkgs/test/make-binary-wrapper/combination/combination.c b/pkgs/test/make-binary-wrapper/combination/combination.c index ae90263c45a5..33a003ca6a9a 100644 --- a/pkgs/test/make-binary-wrapper/combination/combination.c +++ b/pkgs/test/make-binary-wrapper/combination/combination.c @@ -42,10 +42,10 @@ void set_env_prefix(char *env, char *sep, char *prefix) { return; } unsigned long sep_len = strlen(sep); - int n_before = existing_prefix - existing_env; + int n_before = existing_prefix - existing_env - sep_len; assert_success(asprintf(&val, "%s%s%.*s%s", prefix, sep, n_before, existing_env, - existing_prefix + prefix_len + sep_len)); + existing_prefix + prefix_len)); } else { assert_success(asprintf(&val, "%s%s%s", prefix, sep, existing_env)); } diff --git a/pkgs/test/make-binary-wrapper/default.nix b/pkgs/test/make-binary-wrapper/default.nix index 715b28f912e4..7d2e4b1a7cd2 100644 --- a/pkgs/test/make-binary-wrapper/default.nix +++ b/pkgs/test/make-binary-wrapper/default.nix @@ -59,6 +59,7 @@ let "overlength-strings" "prefix" "suffix" + "prefix-dedup-last" ] makeGoldenTest // lib.optionalAttrs (!stdenv.hostPlatform.isDarwin) { cross = diff --git a/pkgs/test/make-binary-wrapper/overlength-strings/overlength-strings.c b/pkgs/test/make-binary-wrapper/overlength-strings/overlength-strings.c index 6a5107d5a4de..0dde8218c3bf 100644 --- a/pkgs/test/make-binary-wrapper/overlength-strings/overlength-strings.c +++ b/pkgs/test/make-binary-wrapper/overlength-strings/overlength-strings.c @@ -42,10 +42,10 @@ void set_env_prefix(char *env, char *sep, char *prefix) { return; } unsigned long sep_len = strlen(sep); - int n_before = existing_prefix - existing_env; + int n_before = existing_prefix - existing_env - sep_len; assert_success(asprintf(&val, "%s%s%.*s%s", prefix, sep, n_before, existing_env, - existing_prefix + prefix_len + sep_len)); + existing_prefix + prefix_len)); } else { assert_success(asprintf(&val, "%s%s%s", prefix, sep, existing_env)); } diff --git a/pkgs/test/make-binary-wrapper/prefix-dedup-last/prefix-dedup-last.c b/pkgs/test/make-binary-wrapper/prefix-dedup-last/prefix-dedup-last.c new file mode 100644 index 000000000000..8f13278a7537 --- /dev/null +++ b/pkgs/test/make-binary-wrapper/prefix-dedup-last/prefix-dedup-last.c @@ -0,0 +1,64 @@ +#define _GNU_SOURCE /* See feature_test_macros(7) */ +#include +#include +#include +#include +#include + +#define assert_success(e) do { if ((e) < 0) { perror(#e); abort(); } } while (0) + +int is_surrounded_by_sep(char *env, char *ptr, unsigned long len, char *sep) { + unsigned long sep_len = strlen(sep); + + // Check left side (if not at start) + if (env != ptr) { + if (ptr - env < sep_len) + return 0; + if (strncmp(sep, ptr - sep_len, sep_len) != 0) { + return 0; + } + } + // Check right side (if not at end) + char *end_ptr = ptr + len; + if (*end_ptr != '\0') { + if (strncmp(sep, ptr + len, sep_len) != 0) { + return 0; + } + } + + return 1; +} + +void set_env_prefix(char *env, char *sep, char *prefix) { + char *existing_env = getenv(env); + if (existing_env) { + char *val; + + char *existing_prefix = strstr(existing_env, prefix); + unsigned long prefix_len = strlen(prefix); + // If the prefix already exists, remove the original + if (existing_prefix && is_surrounded_by_sep(existing_env, existing_prefix, prefix_len, sep)) { + if (existing_env == existing_prefix) { + return; + } + unsigned long sep_len = strlen(sep); + int n_before = existing_prefix - existing_env - sep_len; + assert_success(asprintf(&val, "%s%s%.*s%s", prefix, sep, + n_before, existing_env, + existing_prefix + prefix_len)); + } else { + assert_success(asprintf(&val, "%s%s%s", prefix, sep, existing_env)); + } + assert_success(setenv(env, val, 1)); + free(val); + } else { + assert_success(setenv(env, prefix, 1)); + } +} + +int main(int argc, char **argv) { + putenv("PATH=/usr/bin:/usr/local/bin"); + set_env_prefix("PATH", ":", "/usr/local/bin"); + argv[0] = "/send/me/flags"; + return execv("/send/me/flags", argv); +} diff --git a/pkgs/test/make-binary-wrapper/prefix-dedup-last/prefix-dedup-last.cmdline b/pkgs/test/make-binary-wrapper/prefix-dedup-last/prefix-dedup-last.cmdline new file mode 100644 index 000000000000..ec2d43b44031 --- /dev/null +++ b/pkgs/test/make-binary-wrapper/prefix-dedup-last/prefix-dedup-last.cmdline @@ -0,0 +1,2 @@ +--set PATH /usr/bin:/usr/local/bin \ +--prefix PATH : /usr/local/bin diff --git a/pkgs/test/make-binary-wrapper/prefix-dedup-last/prefix-dedup-last.env b/pkgs/test/make-binary-wrapper/prefix-dedup-last/prefix-dedup-last.env new file mode 100644 index 000000000000..6f8f684af379 --- /dev/null +++ b/pkgs/test/make-binary-wrapper/prefix-dedup-last/prefix-dedup-last.env @@ -0,0 +1,3 @@ +PATH=/usr/local/bin:/usr/bin +CWD=SUBST_CWD +SUBST_ARGV0 diff --git a/pkgs/test/make-binary-wrapper/prefix/prefix.c b/pkgs/test/make-binary-wrapper/prefix/prefix.c index 205ecd0dcaef..a74cb49861ea 100644 --- a/pkgs/test/make-binary-wrapper/prefix/prefix.c +++ b/pkgs/test/make-binary-wrapper/prefix/prefix.c @@ -42,10 +42,10 @@ void set_env_prefix(char *env, char *sep, char *prefix) { return; } unsigned long sep_len = strlen(sep); - int n_before = existing_prefix - existing_env; + int n_before = existing_prefix - existing_env - sep_len; assert_success(asprintf(&val, "%s%s%.*s%s", prefix, sep, n_before, existing_env, - existing_prefix + prefix_len + sep_len)); + existing_prefix + prefix_len)); } else { assert_success(asprintf(&val, "%s%s%s", prefix, sep, existing_env)); }