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.
(In reply to Manu from comment #0) NOTE, there is a typo on all docker run examples: `sh` is missing before `-c`.