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 232bc2b5c3cd3..595574468ffa9 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 ae90263c45a51..33a003ca6a9af 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 715b28f912e49..7d2e4b1a7cd29 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 6a5107d5a4ded..0dde8218c3bf3 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 0000000000000..8f13278a75378 --- /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 0000000000000..ec2d43b44031d --- /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 0000000000000..6f8f684af3795 --- /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 205ecd0dcaef2..a74cb49861eae 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)); }