From a692e776a3ffbadd43f891f94f8849fc240c178f Mon Sep 17 00:00:00 2001 From: Fredrik Fornwall Date: Sun, 14 Jul 2024 19:15:25 +0200 Subject: [PATCH] Do not clean environment when executing /system/bin/sh Cleaning the environment from LD_PRELOAD when executing /system/bin/sh breaks things such as `popen(3)` and `system(3)`. --- .github/workflows/ci.yml | 17 +++++++++++++---- .gitignore | 2 ++ Makefile | 22 ++++++++++++++++------ src/termux-exec.c | 4 +++- tests/call-system-bin-sh.sh | 2 +- tests/call-system-bin-sh.sh-expected | 2 +- tests/fexecve.c | 19 +++++++++++-------- tests/popen.c | 10 ++++++++++ tests/popen.sh | 3 +++ tests/popen.sh-expected | 1 + tests/system-uname.c | 6 ++++++ tests/system-uname.sh | 3 +++ tests/system-uname.sh-expected | 1 + 13 files changed, 71 insertions(+), 21 deletions(-) create mode 100644 tests/popen.c create mode 100755 tests/popen.sh create mode 100644 tests/popen.sh-expected create mode 100644 tests/system-uname.c create mode 100755 tests/system-uname.sh create mode 100644 tests/system-uname.sh-expected diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4d852e2..e4415ab 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,15 +12,24 @@ jobs: steps: - uses: actions/checkout@v4 - uses: Homebrew/actions/setup-homebrew@master + - uses: nttld/setup-ndk@v1 + id: setup-ndk + with: + ndk-version: r26d + link-to-sdk: true - run: brew install clang-format - - run: make CC=clang - - run: make check CC=clang - - run: make unit-test CC=clang + - run: make CC="${ANDROID_NDK_HOME}"/toolchains/llvm/prebuilt/linux-x86_64/bin/aarch64-linux-android30-clang + env: + ANDROID_NDK_HOME: ${{ steps.setup-ndk.outputs.ndk-path }} + - run: make check CLANG_TIDY="${ANDROID_NDK_HOME}/toolchains/llvm/prebuilt/linux-x86_64/bin/clang-tidy --extra-arg=--target=aarch64-linux-android29" + env: + ANDROID_NDK_HOME: ${{ steps.setup-ndk.outputs.ndk-path }} + - run: make unit-test CC=clang HOST_BUILD=1 actionlint: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v4 - name: Download actionlint id: get_actionlint run: bash <(curl https://raw.githubusercontent.com/rhysd/actionlint/main/scripts/download-actionlint.bash) diff --git a/.gitignore b/.gitignore index 72b6172..8caebef 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,5 @@ *.deb test-binary tests/fexecve +tests/popen +tests/system-uname diff --git a/Makefile b/Makefile index 901f06a..e1357c4 100644 --- a/Makefile +++ b/Makefile @@ -1,8 +1,8 @@ CC ?= clang TERMUX_BASE_DIR ?= /data/data/com.termux/files -CFLAGS += -Wall -Wextra -Werror -Wshadow -fvisibility=hidden -std=c17 -Wno-error=tautological-pointer-compare +CFLAGS += -Wall -Wextra -Werror -Wshadow -fvisibility=hidden -std=c17 C_SOURCE := src/termux-exec.c src/exec-variants.c -CLANG_FORMAT := clang-format --sort-includes --style="{ColumnLimit: 120}" $(C_SOURCE) +CLANG_FORMAT := clang-format --sort-includes --style="{ColumnLimit: 120}" $(C_SOURCE) tests/fexecve.c tests/system-uname.c tests/print-argv0.c tests/popen.c CLANG_TIDY ?= clang-tidy ifeq ($(SANITIZE),1) @@ -11,12 +11,22 @@ else CFLAGS += -O2 endif +ifeq ($(HOST_BUILD),1) + CFLAGS += -Wno-error=tautological-pointer-compare +endif + libtermux-exec.so: $(C_SOURCE) $(CC) $(CFLAGS) $(LDFLAGS) $(C_SOURCE) -DTERMUX_PREFIX=\"$(TERMUX_PREFIX)\" -DTERMUX_BASE_DIR=\"$(TERMUX_BASE_DIR)\" -shared -fPIC -o libtermux-exec.so tests/fexecve: tests/fexecve.c $(CC) $(CFLAGS) -DTERMUX_BASE_DIR=\"$(TERMUX_BASE_DIR)\" $< -o $@ +tests/popen: tests/popen.c + $(CC) $(CFLAGS) -DTERMUX_BASE_DIR=\"$(TERMUX_BASE_DIR)\" $< -o $@ + +tests/system-uname: tests/system-uname.c + $(CC) $(CFLAGS) -DTERMUX_BASE_DIR=\"$(TERMUX_BASE_DIR)\" $< -o $@ + $(TERMUX_BASE_DIR)/usr/bin/termux-exec-test-print-argv0: tests/print-argv0.c $(CC) $(CFLAGS) $< -o $@ @@ -31,9 +41,9 @@ uninstall: on-device-tests: make clean - ASAN_OPTIONS=symbolize=0,detect_leaks=0 make SANITIZE=1 on-device-tests-internal + ASAN_OPTIONS=symbolize=0,detect_leaks=0 make on-device-tests-internal -on-device-tests-internal: libtermux-exec.so tests/fexecve $(TERMUX_BASE_DIR)/usr/bin/termux-exec-test-print-argv0 +on-device-tests-internal: libtermux-exec.so tests/fexecve tests/popen tests/system-uname $(TERMUX_BASE_DIR)/usr/bin/termux-exec-test-print-argv0 @LD_PRELOAD=${CURDIR}/libtermux-exec.so ./run-tests.sh format: @@ -43,8 +53,8 @@ check: $(CLANG_FORMAT) --dry-run $(C_SOURCE) $(CLANG_TIDY) -warnings-as-errors='*' $(C_SOURCE) -- -DTERMUX_BASE_DIR=\"$(TERMUX_BASE_DIR)\" -test-binary: $(C_SOURCE) - $(CC) $(CFLAGS) $(LDFLAGS) $(C_SOURCE) -g -fsanitize=address -fno-omit-frame-pointer -DUNIT_TEST=1 -DTERMUX_BASE_DIR=\"$(TERMUX_BASE_DIR)\" -o test-binary +test-binary: src/termux-exec.c src/exec-variants.c + $(CC) $(CFLAGS) $(LDFLAGS) $^ -g -fsanitize=address -fno-omit-frame-pointer -DUNIT_TEST=1 -DTERMUX_BASE_DIR=\"$(TERMUX_BASE_DIR)\" -o test-binary deb: libtermux-exec.so termux-create-package termux-exec-debug.json diff --git a/src/termux-exec.c b/src/termux-exec.c index 453b322..5c4c15c 100644 --- a/src/termux-exec.c +++ b/src/termux-exec.c @@ -350,7 +350,9 @@ __attribute__((visibility("default"))) int execve(const char *executable_path, c char executable_path_resolved_buffer[PATH_MAX]; char const *executable_path_resolved = realpath(executable_path, executable_path_resolved_buffer); char const *path_to_use = executable_path_resolved ? executable_path_resolved : executable_path; - bool wrap_in_linker = (strstr(path_to_use, TERMUX_BASE_DIR) == path_to_use); + bool wrap_in_linker = (strstr(path_to_use, TERMUX_BASE_DIR) == path_to_use) + // /system/bin/sh is fine, it only uses libc++, libc, and libdl. + || (strcmp(path_to_use, "/system/bin/sh") == 0); // Avoid interfering with Android /system software by removing // LD_PRELOAD and LD_LIBRARY_PATH from env if executing something diff --git a/tests/call-system-bin-sh.sh b/tests/call-system-bin-sh.sh index 6cf7a54..0f9f2ce 100755 --- a/tests/call-system-bin-sh.sh +++ b/tests/call-system-bin-sh.sh @@ -1,3 +1,3 @@ #!/bin/sh -/system/bin/sh -c 'echo LD_PRELOAD=$LD_PRELOAD TERMUX_EXEC__PROC_SELF_EXE=$TERMUX_EXEC__PROC_SELF_EXE' +/system/bin/sh -c 'echo TERMUX_EXEC__PROC_SELF_EXE=$TERMUX_EXEC__PROC_SELF_EXE' diff --git a/tests/call-system-bin-sh.sh-expected b/tests/call-system-bin-sh.sh-expected index e9cc445..ea29f62 100644 --- a/tests/call-system-bin-sh.sh-expected +++ b/tests/call-system-bin-sh.sh-expected @@ -1 +1 @@ -LD_PRELOAD= TERMUX_EXEC__PROC_SELF_EXE= +TERMUX_EXEC__PROC_SELF_EXE=/system/bin/sh diff --git a/tests/fexecve.c b/tests/fexecve.c index 64ffc31..bbc682c 100644 --- a/tests/fexecve.c +++ b/tests/fexecve.c @@ -3,12 +3,15 @@ #include int main() { - const char* path_to_echo = TERMUX_BASE_DIR "/usr/bin/sh"; - int fd = open(path_to_echo, O_RDONLY); - if (fd < 0) perror("open"); - char* args[] = {"sh", "-c", "echo hello fexecve", NULL}; - char* env[] = {NULL}; - fexecve(fd, args, env); - perror("fexecve"); - return 0; + const char *path_to_echo = TERMUX_BASE_DIR "/usr/bin/sh"; + int fd = open(path_to_echo, O_RDONLY); + if (fd < 0) { + perror("open"); + return 1; + } + char *args[] = {"sh", "-c", "echo hello fexecve", NULL}; + char *env[] = {NULL}; + fexecve(fd, args, env); + perror("fexecve"); + return 0; } diff --git a/tests/popen.c b/tests/popen.c new file mode 100644 index 0000000..5126ed9 --- /dev/null +++ b/tests/popen.c @@ -0,0 +1,10 @@ +#include + +int main() { + FILE* file = popen("uname", "r"); + char buffer[101]; + fscanf(file, "%100s", buffer); + pclose(file); + printf("%s\n", buffer); + return 0; +} diff --git a/tests/popen.sh b/tests/popen.sh new file mode 100755 index 0000000..bd501fa --- /dev/null +++ b/tests/popen.sh @@ -0,0 +1,3 @@ +#!/bin/sh + +./tests/popen diff --git a/tests/popen.sh-expected b/tests/popen.sh-expected new file mode 100644 index 0000000..9b07567 --- /dev/null +++ b/tests/popen.sh-expected @@ -0,0 +1 @@ +Linux diff --git a/tests/system-uname.c b/tests/system-uname.c new file mode 100644 index 0000000..a220ac6 --- /dev/null +++ b/tests/system-uname.c @@ -0,0 +1,6 @@ +#include + +int main() { + system("uname"); + return 0; +} diff --git a/tests/system-uname.sh b/tests/system-uname.sh new file mode 100755 index 0000000..0d50a11 --- /dev/null +++ b/tests/system-uname.sh @@ -0,0 +1,3 @@ +#!/bin/sh + +./tests/system-uname diff --git a/tests/system-uname.sh-expected b/tests/system-uname.sh-expected new file mode 100644 index 0000000..9b07567 --- /dev/null +++ b/tests/system-uname.sh-expected @@ -0,0 +1 @@ +Linux