top | item 42212700

TIL: Some surprising code execution sources in bash

114 points| nathan_phoenix | 1 year ago |yossarian.net

46 comments

order

mmsc|1 year ago

Unfortunately, there's a lot of gotchas in Bash like this. A lot of them are documented here: https://mywiki.wooledge.org/BashPitfalls, including the `test -v` case, which is #61. Some more code execution pitfalls are documented here: https://mywiki.wooledge.org/BashProgramming/05?action=show&r... including the `-eq` part (under Arithmetic Expansion).

Basically, the -v case was by design, so for `-v 'hash[$key]'`, "$key is expanded before the array subscript evaluation, and then the whole array plus expanded index is evaluated in a second pass". "Newer versions of bash (5.0 and higher) have a assoc_expand_once option which will suppress the multiple evaluations"

Note that the `-v` case doesn't really work the way one may infer from reading the OP:

> $ key='$(cat /etc/passwd > /tmp/pwned)'

> $ [[ -v 'x[$key]' ]]

> bash: $(cat /etc/passwd > /tmp/pwned): syntax error: operand expected (error token is "$(cat /etc/passwd > /tmp/pwned)") *

> [[ -v "${x[$key]}" ]]

> bash: $(cat /etc/passwd > /tmp/pwned): syntax error: operand expected (error token is "$(cat /etc/passwd > /tmp/pwned)")

PhilipRoman|1 year ago

Yuck, I was always instinctively put off by [[, now I finally have some arguments to justify it.

IMO safe shell scripting is kind of dead. I can do it if I really have to, but too many external programs have tricky "convenience" features like interpreting flags after positional parameters, etc.

kreetx|1 year ago

Ack to yuck, but dead.. definitely not. Pretty sure large amounts of shell still get written, mostly due to it being the default scripting interface to the operating system.

voidfunc|1 year ago

So many footguns in bash. When do we finally get serious about ditching this language as an industry in the same way we are about memory safety?

alganet|1 year ago

You underestimate the effort of doing this.

We all want bash gone, but it is an essential piece of infrastructure. The introduction of dash was a huge step in this direction (of ditching bash).

Do you want to help? Try to remove bash from the toolchain bootstrap. It is one of the lowest hanging fruits right now.

jerf|1 year ago

A lot of this behavior is only a major problem if you're putting arbitrary input in, and especially, externally sourced input.

The "good news" is that bash is so full of ways to get command execution that people blow their foot off and get compromised long before these little details are what are compromising their system. People get popped putting in user input at the base string layer where all you have to do is slap down a semi-colon to get arbitrary command execution long before they're getting popped by obscure "test" behaviors.

flanbiscuit|1 year ago

Curious what you use instead of bash? When you spin up a server somewhere, what's the first thing you like to install that replaces what we typically use bash for?

factormeta|1 year ago

Oh no!!! But what about all those Docker files, and k8 clusters!!!

Serious, please view the curled file from a link before piping it to bash/sh.

spiffytech|1 year ago

What's the fix for those code samples?

Shellcheck currently gives Sample 1 a pass. I hope this is something it can be modified to catch.

pizzalife|1 year ago

Honestly, the fix is to only allow alphanumeric input to shellscripts. Anything else invariably fails at some point.

usr1106|1 year ago

The first function one is not particularly well-written, but harmless. The quoting of

   ${num}
is completely useless. Inside [[ bash does not do any word splitting after variable expansion. Double quotes never prevent variable expansion. I am not sure what the author is talking about. Shellcheck is correct to not complain. I stopped reading there.

webstrand|1 year ago

I... don't understand. I thought the whole reason for using [[ and breaking posix compatibility was to prevent just this kind of vulnerability. Why would bash do this.

oneshtein|1 year ago

Instead of `if [[ "${num}" -eq 42 ]]`, bash expects `if [ "${num}" -eq 42 ]` or `if (( num==42 ))`.

tpoacher|1 year ago

From what I understand, based on the premise that this results from switching into 'arithmetic' mode, you don't even need test. The following will also work with the proposed attack:

  function guess () { declare -i num="${1}" ; }
(unless I'm missing something?)

joveian|1 year ago

Why I couldn't guess but an example similar to the article that I tried does not immediately execute (version 5.2.37(1)-release) when indrected through a variable as you show although other aritmetic evaluation does still happen when indirected. You can echo "${num}" and it shows the passed string. If you change it to declare -i num ; num="${1}" then it does immediately execute.

zettabomb|1 year ago

Honestly I just don't write shell scripts anymore, bash or otherwise. By the time any system I use is up, Python is available. I don't know if I've found a true need for shell in anything application level. I'll even fire up a Python shell for something simple like mass renaming files, simply because the string manipulation is so much easier.

IYasha|1 year ago

I have a related question: is integer/"((math))" logic really safer (in bash) than "[normal]"? I usually try hard to use declare -i iMyVar; as many applicable variables as possible. But evaluation of strings is still usually a hellhole... I mean hole hell.

tpoacher|1 year ago

Question: why does the evaluation inside a[] (which does not produce a value) not result in a bad array subscript error in this case?

if you try to evaluate this kind of things as an arithmetic expression directly, it will fail with an error of a bad subscript (mind you, the attack will still work though).

alganet|1 year ago

My first insinct would be to remove the bashisms first:

https://gist.github.com/alganet/a4198158651f3b2dc43ce658052e...

Then, if we run it:

"line 3: test: a[$(cat /etc/passwd > /tmp/pwned)] + 42: integer expression expected"

woodruffw|1 year ago

(Author of the post.)

Yep, this is specifically a bashism (by way of being a kshism). However, it's worth noting that the second variant (`type -v`) will work in `[` and `test`.

(It's also a still a bashism, but IME people don't realize how little of `type` is actually POSIX.)