[26.05] makeBinaryWrapper: fix read past NUL

[ooonea]

Backport of #541663 to staging-26.05 (staging like the original: makeBinaryWrapper changes are mass rebuilds — the master PR #541658 was redirected to staging for the same reason).

set_env_prefix reads past the NUL terminator when the prefixed directory is the last element of the variable's existing value: the removal arithmetic skips a separator that does not exist there, and since environ strings are contiguous, the neighbouring environment string gets spliced into the value. Regression from #451031, so 26.05 has carried it since release. Seen in the wild via gnome-session's wrapper leaving GIO_EXTRA_MODULES=<module dirs>:GTK_PATH=<the entire GTK_PATH value> in the session; reproduced on 26.05 with a wrapper around coreutils env.

Conflict resolution: the pkgs/test/make-binary-wrapper/*/*.c golden files are generated, and the staging versions carry other staging-only generator changes — so the goldens were regenerated on this branch with the patched generator. That also required regenerating suffix/suffix.c, which the original PR did not need to touch on staging but which diverges here. The generator change itself cherry-picked clean.

Tested: all native golden tests (tests.makeBinaryWrapper.*\, including the new prefix-dedup-last) build green on x86_64-linux. The cross test needs an uncached cross toolchain on this branch and was left to Hydra.

Backport prepared with Claude Code (Claude Fable 5).

[ncfavier]

Backport prepared with Claude Code (Claude Fable 5).

🤦

[nixpkgs-commit-check]

This report is automatically generated by the PR / Check / cherry-pick CI workflow.

Some of the commits in this PR require the author's and reviewer's attention.

Sometimes it is not possible to cherry-pick exactly the same patch.
This most frequently happens when resolving merge conflicts.
The range-diff will help to review the resolution of conflicts.

If you need to merge this PR despite the warnings, please dismiss this review shortly before merging.

Warning

Difference between 55876e6 and original d75cae8 may warrant inspection.

Show diff

@@ Commit message
     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.
 
+    (cherry picked from commit d75cae80fa4ab7b5f6f39ddcbbbfbaa1a9183d81)
+
  ## pkgs/by-name/ma/makeBinaryWrapper/make-binary-wrapper.sh ##
 @@ pkgs/by-name/ma/makeBinaryWrapper/make-binary-wrapper.sh: void set_env_prefix(char *env, char *sep, char *prefix) {
          return;
@@ pkgs/by-name/ma/makeBinaryWrapper/make-binary-wrapper.sh: void set_env_prefix(ch
      }
 
  ## pkgs/test/make-binary-wrapper/combination/combination.c ##
-@@ pkgs/test/make-binary-wrapper/combination/combination.c: void set_env_prefix(char *env, char *sep, char *prefix) {
-         return;
-       }
-       unsigned long sep_len = strlen(sep);
--      int n_before = existing_prefix - existing_env;
+@@
+ #include <stdlib.h>
+ #include <assert.h>
+ #include <stdio.h>
++#include <string.h>
+ 
+ #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 = getenv(env);
+-    if (existing) {
+-        char *val;
+-        assert_success(asprintf(&val, "%s%s%s", prefix, sep, existing));
+-        assert_success(setenv(env, val, 1));
+-        free(val);
++  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 + 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, prefix, 1));
++      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));
++  }
  }
  
+ void set_env_suffix(char *env, char *sep, char *suffix) {
+-    char *existing = getenv(env);
+-    if (existing) {
+-        char *val;
+-        assert_success(asprintf(&val, "%s%s%s", existing, sep, suffix));
+-        assert_success(setenv(env, val, 1));
+-        free(val);
++  char *existing_env = getenv(env);
++  if (existing_env) {
++    char *val;
++
++    char *existing_suffix = strstr(existing_env, suffix);
++    unsigned long suffix_len = strlen(suffix);
++    // If the suffix already exists, remove the original
++    if (existing_suffix && is_surrounded_by_sep(existing_env, existing_suffix, suffix_len, sep)) {
++      char *end_ptr = existing_suffix + suffix_len;
++      if (*end_ptr == '\0') {
++        return;
++      }
++      unsigned long sep_len = strlen(sep);
++      int n_before = existing_suffix - existing_env;
++      assert_success(asprintf(&val, "%.*s%s%s%s",
++                              n_before, existing_env,
++                              existing_suffix + suffix_len + sep_len,
++                              sep, suffix));
+     } else {
+-        assert_success(setenv(env, suffix, 1));
++      assert_success(asprintf(&val, "%s%s%s", existing_env, sep, suffix));
+     }
++    assert_success(setenv(env, val, 1));
++    free(val);
++  } else {
++    assert_success(setenv(env, suffix, 1));
++  }
+ }
+ 
+ int main(int argc, char **argv) {
+
  ## pkgs/test/make-binary-wrapper/default.nix ##
 @@ pkgs/test/make-binary-wrapper/default.nix: let
        "overlength-strings"
@@ pkgs/test/make-binary-wrapper/default.nix: let
        cross =
 
  ## pkgs/test/make-binary-wrapper/overlength-strings/overlength-strings.c ##
-@@ pkgs/test/make-binary-wrapper/overlength-strings/overlength-strings.c: void set_env_prefix(char *env, char *sep, char *prefix) {
-         return;
-       }
-       unsigned long sep_len = strlen(sep);
--      int n_before = existing_prefix - existing_env;
+@@
+ #include <stdlib.h>
+ #include <assert.h>
+ #include <stdio.h>
++#include <string.h>
+ 
+ #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 = getenv(env);
+-    if (existing) {
+-        char *val;
+-        assert_success(asprintf(&val, "%s%s%s", prefix, sep, existing));
+-        assert_success(setenv(env, val, 1));
+-        free(val);
++  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 + 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, prefix, 1));
++      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) {
 
  ## pkgs/test/make-binary-wrapper/prefix-dedup-last/prefix-dedup-last.c (new) ##
 @@
@@ pkgs/test/make-binary-wrapper/prefix-dedup-last/prefix-dedup-last.env (new)
 +SUBST_ARGV0
 
  ## pkgs/test/make-binary-wrapper/prefix/prefix.c ##
-@@ pkgs/test/make-binary-wrapper/prefix/prefix.c: void set_env_prefix(char *env, char *sep, char *prefix) {
-         return;
-       }
-       unsigned long sep_len = strlen(sep);
--      int n_before = existing_prefix - existing_env;
+@@
+ #include <stdlib.h>
+ #include <assert.h>
+ #include <stdio.h>
++#include <string.h>
+ 
+ #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 = getenv(env);
+-    if (existing) {
+-        char *val;
+-        assert_success(asprintf(&val, "%s%s%s", prefix, sep, existing));
+-        assert_success(setenv(env, val, 1));
+-        free(val);
++  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 + 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, prefix, 1));
++      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) {
+
+ ## pkgs/test/make-binary-wrapper/suffix/suffix.c ##
+@@
+ #include <stdlib.h>
+ #include <assert.h>
+ #include <stdio.h>
++#include <string.h>
+ 

[...truncated...]

Hint: The full diffs are also available in the runner logs with slightly better highlighting.

explained in PR desc