Bug 14031

Created attachment 9066 [details]
Potential patch for this bug

# Summary

I've encountered a bug in the string substitution logic in shell/ash.c resulting
in segmentation fault. Run this, for example:

```shell
docker run --rm -it busybox:1.33.1-musl -c 'A=$(yes ? | head -n 2047 | tr -d "\n"); echo "${A///}"'
```

This bug is still present in the current master branch (as of dabbeeb7). It was
introduced in da2e46df (somewhere between 1.31.1 and 1.32.1). I'm also providing
a potential patch.

Besides that, I think it might be beneficial to use fuzzing to discover similar
bugs in the shell string substitution feature.

NOTE: Possibly a duplicate of https://bugs.busybox.net/show_bug.cgi?id=13556


# The bug

The bug is related to the shell string substitution logic implemented in
shell/ash.c. It's only seen in very particular circumstances, namely:
- Using musl
- Performing the substitution within quotes
- Having relatively long content in the variable being substituted
- Having special characters (like "*" or "?") in the content of the variable
  being substituted

## How to reproduce

I'm using busybox version 1.33.1 under Docker for illustrative purposes:

```shell
# Fails (musl + ? character + 2047 bytes + string substitution + quoted expansion)
docker run --rm -it busybox:1.33.1-musl -c 'A=$(yes ? | head -n 2047 | tr -d "\n"); echo "${A///}"'

# Works (shorter content)
docker run --rm -it busybox:1.33.1-musl -c 'A=$(yes ? | head -n 20 | tr -d "\n"); echo "${A///}"'

# Works (longer content)
docker run --rm -it busybox:1.33.1-musl -c 'A=$(yes ? | head -n 5000 | tr -d "\n"); echo "${A///}"'

# Works (unquoted expansion)
docker run --rm -it busybox:1.33.1-musl -c 'A=$(yes ? | head -n 2047 | tr -d "\n"); echo ${A///}'

# Works (different character)
docker run --rm -it busybox:1.33.1-musl -c 'A=$(yes x | head -n 2047 | tr -d "\n"); echo "${A///}"'

# Works (no string substitution)
docker run --rm -it busybox:1.33.1-musl -c 'A=$(yes ? | head -n 2047 | tr -d "\n"); echo "${A}"'

# Works (glibc)
docker run --rm -it busybox:1.33.1-glibc sh -c 'A=$(yes ? | head -n 2047 | tr -d "\n"); echo "${A///}"'

# Works (uclibc)
docker run --rm -it busybox:1.33.1-uclibc sh -c 'A=$(yes ? | head -n 2047 | tr -d "\n"); echo "${A///}"'
```

It might be also interesting to run this, which shows how it segfaults only for
certain sizes (the first one being i=883):

```shell
for i in $(seq 1 2500); do echo $i;
  [[ $(./busybox_unstripped sh -c 'A=$(yes ? | head -n '$i' | tr -d "\n"); echo "${A///}"' | wc -c) -eq $(( i + 1)) ]] || echo "Failed";
done
```

## When was the bug introduced

The following `git bisect` operation

```shell
git checkout dabbeeb7; git bisect start; git bisect bad; git bisect good 1_31_1; git bisect run ../bisect.sh
```

using this bisect.sh script as test for the bug

```shell
#/bin/sh
set -e
make clean distclean > /dev/null
yes "" | make oldconfig > /dev/null
make CC=musl-gcc -j12 > /dev/null 2>&1
set +e
./busybox_unstripped sh -c 'A=$(yes ? | head -n 2047 | tr -d "\n"); echo "${A///}"' > /dev/null
[ $? -eq 0 ]
```

yields that the bug exists since commit da2e46df.


# The potential patch

See attachment.

I say *potential* because, even though it seems to fix the issue indeed, I don't
know enough about the implementation to be confident that this is an appropriate
fix.

I'm adding a test case mostly for illustrative purposes as well, but it feels
like it might be better tested with fuzzing and not with a concrete input.