Skip to content

Commit

Permalink
[sb2] prepare_exec: Do not discard changes to environment. JB#52287
Browse files Browse the repository at this point in the history
Initially, my_envp equals to *new_envp, then it is modified. Those
modifications are lost when *new_envp is used after that.

The (possibly) discarted modifications to env. variables affect
SBOX_DISABLE_MAPPING, SBOX_DISABLE_ARGVENVP and __SB2_REAL_BINARYNAME.

Regarding SBOX_DISABLE_MAPPING, consider a host executable being
executed by sb2 and trying to execute another host executable. With
mapping not disabled, the other executable may be mapped to a different
executable under target/tools root instead. In most cases this is likely
to happen unnoticed. It breaks when there is an incompatibility between
those, e.g., when the host executable is a GCC compiler trying to
execute some of its internal tools - an i486 host compiler may end up
executing an arm assembler this way.

Trying to explain why *new_envp was used here, I don't believe it was a
typo. At the same time I don't believe it was intentional to change the
logic this way. I can't think of a reason to discard changes to the
afforementioned env. vars. To me it seems inevitable to keep them set.
Also, change was introduced with commit
3f042f3 (Execs: Postprocess native
dynamic binaries with C code) whose only objective seems to be
reimplementation of some Lua code in C.  Maybe it was done to deal with
some compiler bug and no one noticed it breaks things, as this only
breaks in rather special cases like the one mentioned above.
  • Loading branch information
martyone committed Dec 17, 2020
1 parent c0c79d3 commit be954f5
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions scratchbox2/execs/sb_exec.c
Expand Up @@ -1535,15 +1535,15 @@ static int prepare_exec(const char *exec_fn_name,
exec_policy_name,
&mapped_file, &my_file, binaryname,
(const char **)my_argv, &my_new_argv,
(const char **)*new_envp, &my_new_envp,
(const char **)my_envp, &my_new_envp,
"target");
} else {
postprocess_result = exec_postprocess_native_executable(
exec_policy_name,
&mapped_file, &my_file, binaryname,
&info,
(const char **)my_argv, &my_new_argv,
(const char **)*new_envp, &my_new_envp);
(const char **)my_envp, &my_new_envp);
}
my_envp = (char**)my_new_envp; /* FIXME */
my_argv = (char**)my_new_argv; /* FIXME */
Expand All @@ -1570,7 +1570,7 @@ static int prepare_exec(const char *exec_fn_name,
exec_policy_name,
&mapped_file, &my_file, binaryname,
(const char **)my_argv, &my_new_argv,
(const char **)*new_envp, &my_new_envp,
(const char **)my_envp, &my_new_envp,
"native");
my_envp = (char**)my_new_envp; /* FIXME */
my_argv = (char**)my_new_argv; /* FIXME */
Expand Down Expand Up @@ -1609,7 +1609,7 @@ static int prepare_exec(const char *exec_fn_name,
exec_policy_name,
&mapped_file, &my_file, binaryname,
(const char **)my_argv, &my_new_argv,
(const char **)*new_envp, &my_new_envp);
(const char **)my_envp, &my_new_envp);
my_envp = (char**)my_new_envp; /* FIXME */
my_argv = (char**)my_new_argv; /* FIXME */
}
Expand Down Expand Up @@ -1639,7 +1639,7 @@ static int prepare_exec(const char *exec_fn_name,
postprocess_result = exec_postprocess_cpu_transparency_executable(
exec_policy_name,
&mapped_file, &my_file, binaryname,
(const char **)my_argv, &my_new_argv, (const char **)*new_envp, &my_new_envp,
(const char **)my_argv, &my_new_argv, (const char **)my_envp, &my_new_envp,
"target");
my_envp = (char**)my_new_envp; /* FIXME */
my_argv = (char**)my_new_argv; /* FIXME */
Expand Down

0 comments on commit be954f5

Please sign in to comment.