Bugzilla – Bug 765
$var expanded too early when using sudo -i.
Last modified: 2017-05-04 18:38:01 MDT
Bug 413 has mostly been fixed, but the code in parse_args.c ("quote potential meta characters") specifically excludes $. This means that $var references in the command can get expanded too early. I would expect the following two commands to have the same output ("foo is xxx") but that is not the case: % sudo -i bash -c 'FOO=xxx; echo foo is $FOO' foo is % sudo bash -c 'FOO=xxx; echo foo is $FOO' foo is xxx Changing: /* quote potential meta characters */ if (!isalnum((unsigned char)*src) && *src != '_' && *src != '-' && *src != '$') to: /* quote potential meta characters */ if (!isalnum((unsigned char)*src) && *src != '_' && *src != '-') seems to fix the issue.
Escaping of '$' was excluded as a fix for Bug 564. Unfortuately, there is disagreement on what should be expanded when running commands via "sudo -i".
I believe that Bug 564 should have been marked "invalid". However, if we do accept that it was a real bug, even then, the fix was a buggy half-measure.The fix to Bug 564 was a half-measure: # using C-style strings and arrays; the output of strace # works in the trivial case ["sudo", "-i", "/bin/echo", "$USER"] -> ["-bash", "--login", "-c", "\\/bin\\/echo $USER"] # but confusingly fails if we try deviating from that ["sudo", "-i", "/bin/echo", "${USER}"] -> ["-bash", "--login", "-c", "\\/bin\\/echo $\\{USER\\}"] ["sudo", "-i", "/bin/echo", "$@"] -> ["-bash", "--login", "-c", "\\/bin\\/echo $\\@"] Ok, so what went wrong, and why I believe that Bug 564 was invalid: There are two possible ways that the command could be passed around: 1. A list of strings to use as arguments; the same kind of thing that you would pass to execve(2). 2. A single string to be evaluated by the shell; the same kind of thing that you would pass to system(3). Now, internally, sudo needs #1 normally, but needs #2 if given `-s` or `-i`. So, the "rewrite argv" routine in parse_args.c was written to transform #1 in #2 when int needs #2 internally. This way, it could expect to always consume #1 from the outside world; it would never consume #2. Then, Bug 654 was opened by Bert, who expecting what sudo consumes to have attributes of #2. They seemed to have a little confused thinking. It wasn't totally unreasonable of them; the man page ambiguously suggests that with `-s` or `-i` that it might take #2. However, it seems odd to me that they would expect `sudo -i echo '$USER'` to expand $USER, when `sudo echo '$USER'` (without the `-i` flag) it doesn't. It really sounds to me like Bert just had code that happened to rely on Bug #413, and then filed a bug that his code broke when #413 was fixed. Anyway, the "fix" to Bug 654 switched `-i` and `-s` from consuming #1 to consuming some weird mashup of #1 and #2 that doesn't really make sense to anybody, but satisfied Bert's testcase. Now, the downside is: There's a bug here; and there's no way to fix it without breaking someone's code, as someone out there (and, err, (in my opinion) Bert in particular) is relying on the buggy behavior.