jmtd commented on pull request #556:
URL: https://github.com/apache/maven/pull/556#issuecomment-931189840
Hi @michael-o thanks for your comments. The approach I used was lifted
straight from the opengroup.org URI in the PR body/commit. However, I realised
last night that I'd made a mistake in my local testing, so I have revised and
force pushed.
To answer your questions
> Do you assume that unalias could be an alias as well?
Yes, and the (escaped) backslash prefix mitigates that,
> Why are you unset all aliases? Do you assume java(1) to be an alias?
Simply because it's easier than specifying the ones to unset. The sub-shell
context inside the backticks doesn't leak out to the wider init script, so that
`unset` doesn't take effect later on in the script.
> unset is to use the shell builtin only?
Yes. At least to attempt to!
The mistake in my local testing related to `unalias`. I found that it's just
not reliable in all situations, and using the backslash prefix is preferable.
Here's a script demonstrating an adversarial environment and the results for a
bunch of shells:
```
$ cat adversarial.sh
command() { echo INTERCEPTED 2; }
alias command="echo INTERCEPTED 1"
echo "`\\unset -f command; \\command -v java`"
$ for shell in bash 'bash --posix' posh dash zsh mksh 'busybox sh'; do echo
-n "$shell: "; $shell adversarial.sh; done
bash: /usr/bin/java
bash --posix: /usr/bin/java
posh: adversarial.sh:2: alias: not found
/usr/bin/java
dash: /usr/bin/java
zsh: /usr/bin/java
mksh: /usr/bin/java
busybox sh: /usr/bin/java
```
The situation that I am aware of that isn't covered is if the adversarial
environment has shadowed `unset`. This is a syntax error in most of the shells
above (including bash in --posix mode) but is possible in bash and zsh. The
mitigation would be to use `builtin`. However, that's a shell extension and not
in POSIX. And, you can shadow `builtin` too! I haven't figured out a defence
against that. Having said that, my reading of [the relevant
document](https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_01_01)
is that shell built-ins should have precedence over functions when resolving
names anyway.
The mitigation is this init script ends up prefixed with `#!/bin/sh`, which
if provided by bash, triggers POSIX mode. I've tested this patch on a Debian
system where `/bin/sh` is provided by `dash`, and a RHEL system where it's
provided by `bash` (same on macos).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
