From 4e55d34535ae278b6130cba43edab6fd6c95e67d Mon Sep 17 00:00:00 2001 From: Tobias Bergkvist Date: Wed, 1 Dec 2021 22:49:20 +0100 Subject: [PATCH] Add assertValidEnvName and check that variable name is valid during code generation. Add assert_success, and assert that setenv/unsetenv succeeds to crash if they don't --- .../setup-hooks/make-binary-wrapper.sh | 28 +++++++++++++++---- pkgs/test/make-binary-wrapper/combination.c | 9 ++++-- pkgs/test/make-binary-wrapper/default.nix | 1 + pkgs/test/make-binary-wrapper/env.c | 7 +++-- pkgs/test/make-binary-wrapper/invalid-env.c | 18 ++++++++++++ pkgs/test/make-binary-wrapper/prefix.c | 7 +++-- pkgs/test/make-binary-wrapper/suffix.c | 5 +++- 7 files changed, 62 insertions(+), 13 deletions(-) create mode 100644 pkgs/test/make-binary-wrapper/invalid-env.c diff --git a/pkgs/build-support/setup-hooks/make-binary-wrapper.sh b/pkgs/build-support/setup-hooks/make-binary-wrapper.sh index d7b888292d58..90b9576740b1 100644 --- a/pkgs/build-support/setup-hooks/make-binary-wrapper.sh +++ b/pkgs/build-support/setup-hooks/make-binary-wrapper.sh @@ -64,7 +64,7 @@ makeDocumentedCWrapper() { # ARGS: same as makeBinaryWrapper makeCWrapper() { local argv0 n params cmd main flagsBefore flags executable params length - local uses_prefix uses_suffix uses_concat3 uses_assert + local uses_prefix uses_suffix uses_assert uses_assert_success uses_concat3 executable=$(escapeStringLiteral "$1") params=("$@") length=${#params[*]} @@ -80,12 +80,14 @@ makeCWrapper() { --set-default) cmd=$(setDefaultEnv "${params[n + 1]}" "${params[n + 2]}") main="$main $cmd"$'\n' + uses_assert_success=1 n=$((n + 2)) [ $n -ge "$length" ] && main="$main #error makeCWrapper: $p takes 2 arguments"$'\n' ;; --unset) cmd=$(unsetEnv "${params[n + 1]}") main="$main $cmd"$'\n' + uses_assert_success=1 n=$((n + 1)) [ $n -ge "$length" ] && main="$main #error makeCWrapper: $p takes 1 argument"$'\n' ;; @@ -94,6 +96,7 @@ makeCWrapper() { main="$main $cmd"$'\n' uses_prefix=1 uses_concat3=1 + uses_assert_success=1 uses_assert=1 n=$((n + 3)) [ $n -ge "$length" ] && main="$main #error makeCWrapper: $p takes 3 arguments"$'\n' @@ -103,6 +106,7 @@ makeCWrapper() { main="$main $cmd"$'\n' uses_suffix=1 uses_concat3=1 + uses_assert_success=1 uses_assert=1 n=$((n + 3)) [ $n -ge "$length" ] && main="$main #error makeCWrapper: $p takes 3 arguments"$'\n' @@ -133,6 +137,8 @@ makeCWrapper() { printf '%s\n' "#include " [ -z "$uses_concat3" ] || printf '%s\n' "#include " [ -z "$uses_assert" ] || printf '%s\n' "#include " + [ -z "$uses_assert_success" ] || printf '%s\n' "#include " + [ -z "$uses_assert_success" ] || printf '\n%s\n' "#define assert_success(e) do { if ((e) < 0) { perror(#e); exit(1); } } while (0)" [ -z "$uses_concat3" ] || printf '\n%s\n' "$(concat3Fn)" [ -z "$uses_prefix" ] || printf '\n%s\n' "$(setEnvPrefixFn)" [ -z "$uses_suffix" ] || printf '\n%s\n' "$(setEnvSuffixFn)" @@ -167,6 +173,7 @@ setEnvPrefix() { sep=$(escapeStringLiteral "$2") val=$(escapeStringLiteral "$3") printf '%s' "set_env_prefix(\"$env\", \"$sep\", \"$val\");" + assertValidEnvName "$1" } # suffix ENV SEP VAL @@ -176,6 +183,7 @@ setEnvSuffix() { sep=$(escapeStringLiteral "$2") val=$(escapeStringLiteral "$3") printf '%s' "set_env_suffix(\"$env\", \"$sep\", \"$val\");" + assertValidEnvName "$1" } # setEnv KEY VALUE @@ -184,6 +192,7 @@ setEnv() { key=$(escapeStringLiteral "$1") value=$(escapeStringLiteral "$2") printf '%s' "putenv(\"$key=$value\");" + assertValidEnvName "$1" } # setDefaultEnv KEY VALUE @@ -191,14 +200,16 @@ setDefaultEnv() { local key value key=$(escapeStringLiteral "$1") value=$(escapeStringLiteral "$2") - printf '%s' "setenv(\"$key\", \"$value\", 0);" + printf '%s' "assert_success(setenv(\"$key\", \"$value\", 0));" + assertValidEnvName "$1" } # unsetEnv KEY unsetEnv() { local key key=$(escapeStringLiteral "$1") - printf '%s' "unsetenv(\"$key\");" + printf '%s' "assert_success(unsetenv(\"$key\"));" + assertValidEnvName "$1" } # Put the entire source code into const char* SOURCE_CODE to make it readable after compilation. @@ -220,6 +231,13 @@ escapeStringLiteral() { printf '%s' "$result" } +assertValidEnvName() { + case "$1" in + *=*) printf '\n%s\n' " #error Illegal environment variable name \`$1\` (cannot contain \`=\`)";; + "") printf '\n%s\n' " #error Environment variable name can't be empty.";; + esac +} + concat3Fn() { printf '%s' "\ char *concat3(char *x, char *y, char *z) { @@ -242,7 +260,7 @@ setEnvPrefixFn() { void set_env_prefix(char *env, char *sep, char *val) { char *existing = getenv(env); if (existing) val = concat3(val, sep, existing); - setenv(env, val, 1); + assert_success(setenv(env, val, 1)); if (existing) free(val); } " @@ -253,7 +271,7 @@ setEnvSuffixFn() { void set_env_suffix(char *env, char *sep, char *val) { char *existing = getenv(env); if (existing) val = concat3(existing, sep, val); - setenv(env, val, 1); + assert_success(setenv(env, val, 1)); if (existing) free(val); } " diff --git a/pkgs/test/make-binary-wrapper/combination.c b/pkgs/test/make-binary-wrapper/combination.c index 5e4e1168f4a0..a4082ac4aea0 100644 --- a/pkgs/test/make-binary-wrapper/combination.c +++ b/pkgs/test/make-binary-wrapper/combination.c @@ -10,6 +10,9 @@ #include #include #include +#include + +#define assert_success(e) do { if ((e) < 0) { perror(#e); exit(1); } } while (0) char *concat3(char *x, char *y, char *z) { int xn = strlen(x); @@ -27,19 +30,19 @@ char *concat3(char *x, char *y, char *z) { void set_env_prefix(char *env, char *sep, char *val) { char *existing = getenv(env); if (existing) val = concat3(val, sep, existing); - setenv(env, val, 1); + assert_success(setenv(env, val, 1)); if (existing) free(val); } void set_env_suffix(char *env, char *sep, char *val) { char *existing = getenv(env); if (existing) val = concat3(existing, sep, val); - setenv(env, val, 1); + assert_success(setenv(env, val, 1)); if (existing) free(val); } int main(int argc, char **argv) { - setenv("MESSAGE", "HELLO", 0); + assert_success(setenv("MESSAGE", "HELLO", 0)); set_env_prefix("PATH", ":", "/usr/bin/"); set_env_suffix("PATH", ":", "/usr/local/bin/"); putenv("MESSAGE2=WORLD"); diff --git a/pkgs/test/make-binary-wrapper/default.nix b/pkgs/test/make-binary-wrapper/default.nix index 72e7fb094a48..1972b03da25b 100644 --- a/pkgs/test/make-binary-wrapper/default.nix +++ b/pkgs/test/make-binary-wrapper/default.nix @@ -29,6 +29,7 @@ let env = makeGoldenTest { name = "env"; filename = ./env.c; }; prefix = makeGoldenTest { name = "prefix"; filename = ./prefix.c; }; suffix = makeGoldenTest { name = "suffix"; filename = ./suffix.c; }; + invalid-env = makeGoldenTest { name = "invalid-env"; filename = ./invalid-env.c; }; }; in runCommand "make-binary-wrapper-test" { passthru = tests; diff --git a/pkgs/test/make-binary-wrapper/env.c b/pkgs/test/make-binary-wrapper/env.c index 89f1f496b349..a1b50b1c3630 100644 --- a/pkgs/test/make-binary-wrapper/env.c +++ b/pkgs/test/make-binary-wrapper/env.c @@ -6,11 +6,14 @@ #include #include +#include + +#define assert_success(e) do { if ((e) < 0) { perror(#e); exit(1); } } while (0) int main(int argc, char **argv) { putenv("PART1=HELLO"); - setenv("PART2", "WORLD", 0); - unsetenv("SOME_OTHER_VARIABLE"); + assert_success(setenv("PART2", "WORLD", 0)); + assert_success(unsetenv("SOME_OTHER_VARIABLE")); putenv("PART3=\"!!\n\""); argv[0] = "/hello/world"; return execv("/hello/world", argv); diff --git a/pkgs/test/make-binary-wrapper/invalid-env.c b/pkgs/test/make-binary-wrapper/invalid-env.c new file mode 100644 index 000000000000..02f7cd01a594 --- /dev/null +++ b/pkgs/test/make-binary-wrapper/invalid-env.c @@ -0,0 +1,18 @@ +// makeCWrapper /bad/env/example \ + --set "=" "TEST1" \ + --set-default "" "TEST2" + +#include +#include +#include + +#define assert_success(e) do { if ((e) < 0) { perror(#e); exit(1); } } while (0) + +int main(int argc, char **argv) { + putenv("==TEST1"); + #error Illegal environment variable name `=` (cannot contain `=`) + assert_success(setenv("", "TEST2", 0)); + #error Environment variable name can't be empty. + argv[0] = "/bad/env/example"; + return execv("/bad/env/example", argv); +} \ No newline at end of file diff --git a/pkgs/test/make-binary-wrapper/prefix.c b/pkgs/test/make-binary-wrapper/prefix.c index fa333013cd02..8f7dd9b679c4 100644 --- a/pkgs/test/make-binary-wrapper/prefix.c +++ b/pkgs/test/make-binary-wrapper/prefix.c @@ -6,6 +6,9 @@ #include #include #include +#include + +#define assert_success(e) do { if ((e) < 0) { perror(#e); exit(1); } } while (0) char *concat3(char *x, char *y, char *z) { int xn = strlen(x); @@ -23,7 +26,7 @@ char *concat3(char *x, char *y, char *z) { void set_env_prefix(char *env, char *sep, char *val) { char *existing = getenv(env); if (existing) val = concat3(val, sep, existing); - setenv(env, val, 1); + assert_success(setenv(env, val, 1)); if (existing) free(val); } @@ -32,4 +35,4 @@ int main(int argc, char **argv) { set_env_prefix("PATH", ":", "/usr/local/bin/"); argv[0] = "/path/to/executable"; return execv("/path/to/executable", argv); -} \ No newline at end of file +} diff --git a/pkgs/test/make-binary-wrapper/suffix.c b/pkgs/test/make-binary-wrapper/suffix.c index a299f1fa0bd6..17160465b41e 100644 --- a/pkgs/test/make-binary-wrapper/suffix.c +++ b/pkgs/test/make-binary-wrapper/suffix.c @@ -6,6 +6,9 @@ #include #include #include +#include + +#define assert_success(e) do { if ((e) < 0) { perror(#e); exit(1); } } while (0) char *concat3(char *x, char *y, char *z) { int xn = strlen(x); @@ -23,7 +26,7 @@ char *concat3(char *x, char *y, char *z) { void set_env_suffix(char *env, char *sep, char *val) { char *existing = getenv(env); if (existing) val = concat3(existing, sep, val); - setenv(env, val, 1); + assert_success(setenv(env, val, 1)); if (existing) free(val); }