这是indexloc提供的服务，不要输入任何密码
Skip to content

Commit 3d7eeca

Browse files
agnostic-apolloagnostic-apollo
committed
Fixed: Abort execve() with a debug error if argv[0] length is >= 128 on Android < 6 instead of letting process fail post execve() without an error
For Android `< 6`, the length must not be `>= 128` for the `argv[0]` string of an `execve()` call or library path of a `dlopen()` call. The `soinfo_alloc()` function in `linker.cpp` of Android `/system/bin/linker*` that loaded the `soinfo` of a library/executable had a `SOINFO_NAME_LEN=128` limit on the path/name passed to it before aae859cc, after which it was increased to `PATH_MAX`. Earlier, if path passed was `>= 128`, then `library name "<library_name>" too long` error would occur. Before dcaef371, the `__linker_init_post_relocation()` function also passed `argv[0]` as executable path to `soinfo_alloc()` function to load its `soinfo`, instead of the actual absolute path of the executable. So before aae859cc, if either length was `>= 128`, then the process would abort with exit code `1`. Note that the `execve()` call itself will not fail, failure occurs before `main()` is called. The limit also applies to the interpreter defined in scripts, as interpreter is passed as `argv[0]` during execution. Both fixes are only available in Android `>= 6`. For earlier versions like Android 5, the path for executables and libraries must be kept below the limit. However, for executables, to allow execution, the `argv[0]` can be shortened even if executable path is longer, or by first changing current working directory to executable's parent directory and then executing it with a relative path. See also `TERMUX__PREFIX_DIR___MAX_LEN`, `TERMUX__PREFIX__BIN_FILE___SAFE_MAX_LEN`, `FILE_HEADER__BINPRM_BUF_SIZE` and `TERMUX__FILE_HEADER__BUFFER_SIZE`. ``` LD_DEBUG=3 /data/data/com.termux/files/usr/libexec/installed-tests/termux-exec/lib/termux-exec_nos_c_tre/scripts/termux/api/termux_exec/ld_preload/direct/exec/files/print-args-binary arg1; echo $? 1 linker W [ android linker & debugger ] linker D DEBUG: library name "/data/data/com.termux/files/usr/libexec/installed-tests/termux-exec/lib/termux-exec_nos_c_tre/scripts/termux/api/termux_exec/ld_preload/direct/exec/files/print-args-binary" too long ``` ``` (exec -a print-args-binary /data/data/com.termux/files/usr/libexec/installed-tests/termux-exec/lib/termux-exec_nos_c_tre/scripts/termux/api/termux_exec/ld_preload/direct/exec/files/print-args-binary arg1); echo $? arg1 0 ``` ``` (cd /data/data/com.termux/files/usr/libexec/installed-tests/termux-exec/lib/termux-exec_nos_c_tre/scripts/termux/api/termux_exec/ld_preload/direct/exec/files && ./print-args-binary arg1); echo $? arg1 0 ``` - https://cs.android.com/android/_/android/platform/bionic/+/dcaef371 - https://cs.android.com/android/_/android/platform/bionic/+/aae859cc - termux/termux-app#213 Credits to @Grimler91 for finding that it was path length that was causing execution failure for binaries.
1 parent ad67e02 commit 3d7eeca

File tree

5 files changed

+222
-37
lines changed

5 files changed

+222
-37
lines changed

‎Makefile‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,11 @@ override LIBTERMUX_EXEC__NOS__C__CPPFLAGS := \
201201
override LIBTERMUX_EXEC__NOS__C__TESTS_BUILD_OUTPUT_DIR := $(TESTS_BUILD_OUTPUT_DIR)/lib/termux-exec_nos_c_tre
202202

203203

204+
ifneq ($(LIBTERMUX_EXEC__NOS__C__EXECVE_CALL__CHECK_ARGV0_BUFFER_OVERFLOW),1)
205+
override LIBTERMUX_EXEC__NOS__C__EXECVE_CALL__CHECK_ARGV0_BUFFER_OVERFLOW := 0
206+
endif
207+
208+
204209

205210
override TERMUX_EXEC__MAIN_APP__TESTS_BUILD_OUTPUT_DIR := $(TESTS_BUILD_OUTPUT_DIR)/app/main
206211

@@ -284,6 +289,7 @@ build-libtermux-exec_nos_c_tre:
284289
mkdir -p "$$(dirname "$(TMP_BUILD_OUTPUT_DIR)/$$source_file")" || exit $$?; \
285290
$(CC) -c $(CFLAGS) $(LIBTERMUX_EXEC__NOS__C__CPPFLAGS) \
286291
$(TERMUX__CONSTANTS__MACRO_FLAGS) \
292+
-DLIBTERMUX_EXEC__NOS__C__EXECVE_CALL__CHECK_ARGV0_BUFFER_OVERFLOW=$(LIBTERMUX_EXEC__NOS__C__EXECVE_CALL__CHECK_ARGV0_BUFFER_OVERFLOW) \
287293
-fPIC -fvisibility=default \
288294
-o $(TMP_BUILD_OUTPUT_DIR)/"$$(echo "$$source_file" | sed -E "s/(.*)\.c$$/\1.o/")" \
289295
"$$source_file" || exit $$?; \

‎lib/termux-exec_nos_c_tre/include/termux/termux_exec__nos__c/v1/termux/api/termux_exec/ld_preload/direct/exec/ExecIntercept.h‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,65 @@ extern "C" {
5555
* - https://github.com/bminor/bash/blob/bash-5.2/execute_cmd.c#L5964
5656
* - https://github.com/bminor/bash/blob/bash-5.2/execute_cmd.c#L5988
5757
* - https://github.com/bminor/bash/blob/bash-5.2/execute_cmd.c#L6048
58+
*
59+
*
60+
*
61+
* For Android `< 6`, the length must not be `>= 128` for the `argv[0]`
62+
* string of an `execve()` call or library path of a `dlopen()` call.
63+
*
64+
* The `soinfo_alloc()` function in `linker.cpp` of Android `/system/bin/linker*`
65+
* that loaded the `soinfo` of a library/executable had a `SOINFO_NAME_LEN=128`
66+
* limit on the path/name passed to it before aae859cc, after which it
67+
* was increased to `PATH_MAX`. Earlier, if path passed was `>= 128`,
68+
* then `library name "<library_name>" too long` error would occur.
69+
*
70+
* Before dcaef371, the `__linker_init_post_relocation()` function also
71+
* passed `argv[0]` as executable path to `soinfo_alloc()` function to
72+
* load its `soinfo`, instead of the actual absolute path of the
73+
* executable. So before aae859cc, if either length was `>= 128`, then
74+
* the process would abort with exit code `1`. Note that the `execve()`
75+
* call itself will not fail, failure occurs before `main()` is called.
76+
* The limit also applies to the interpreter defined in scripts, as
77+
* interpreter is passed as `argv[0]` during execution.
78+
*
79+
* Both fixes are only available in Android `>= 6`. For earlier
80+
* versions like Android 5, the path for executables and libraries
81+
* must be kept below the limit. However, for executables, to allow
82+
* execution, the `argv[0]` can be shortened even if executable path
83+
* is longer, or by first changing current working directory to
84+
* executable's parent directory and then executing it with a relative
85+
* path.
86+
*
87+
* ```
88+
* LD_DEBUG=3 /data/data/com.termux/files/usr/libexec/installed-tests/termux-exec/lib/termux-exec_nos_c_tre/scripts/termux/api/termux_exec/ld_preload/direct/exec/files/print-args-binary arg1; echo $?
89+
* 1
90+
*
91+
* # Logcat
92+
* linker W [ android linker & debugger ]
93+
* linker D DEBUG: library name "/data/data/com.termux/files/usr/libexec/installed-tests/termux-exec/lib/termux-exec_nos_c_tre/scripts/termux/api/termux_exec/ld_preload/direct/exec/files/print-args-binary" too long
94+
* ```
95+
*
96+
* ```
97+
* (exec -a print-args-binary /data/data/com.termux/files/usr/libexec/installed-tests/termux-exec/lib/termux-exec_nos_c_tre/scripts/termux/api/termux_exec/ld_preload/direct/exec/files/print-args-binary arg1); echo $?
98+
* arg1
99+
* 0
100+
* ```
101+
*
102+
* ```
103+
* (cd /data/data/com.termux/files/usr/libexec/installed-tests/termux-exec/lib/termux-exec_nos_c_tre/scripts/termux/api/termux_exec/ld_preload/direct/exec/files && ./print-args-binary arg1); echo $?
104+
* arg1
105+
* 0
106+
* ```
107+
*
108+
* - https://cs.android.com/android/_/android/platform/bionic/+/dcaef371
109+
* - https://cs.android.com/android/_/android/platform/bionic/+/aae859cc
110+
* - https://github.com/termux/termux-app/issues/213
111+
*
112+
* See also `TERMUX__PREFIX__BIN_FILE___SAFE_MAX_LEN` in
113+
* https://github.com/termux/termux-core-package/blob/master/lib/termux-core_nos_c_tre/include/termux/termux_core__nos__c/v1/termux/file/TermuxFile.h
114+
*
115+
* **See Also:**
116+
* - https://github.com/termux/termux-packages/wiki/Termux-file-system-layout#file-path-limits
58117
*/
59118

60119
/**
@@ -276,6 +335,31 @@ int modifyExecArgs(char *const *argv, const char ***newArgvPointer,
276335

277336

278337

338+
/**
339+
* Check if `argv[0]` length is `>= 128` on Android `< 6` as commands
340+
* will fail with exit code 1 without any error on stderr,
341+
* but with the `library name "<library_name>" too long` error in
342+
* `logcat` if linker debugging is enabled.
343+
*
344+
* See comment at top of this file.
345+
*
346+
* @param argv The current arguments pointer.
347+
* @param origExecutablePath The originnal executable path passed to
348+
* `execve()`.
349+
* @param executablePath The **normalized** executable or interpreter
350+
* path that will actually be executed.
351+
* @param processedExecutablePath The **normalized** executable path
352+
* that was passed to `execve()`.
353+
* @param interpreterSet Whether a interpreter is set in the executable
354+
* file.
355+
* @return Returns `0` `argv[0]` length is `< 128` or running on
356+
* Android `>= 6`, otherwise `-1` with errno set to `ENAMETOOLONG` if
357+
* buffer overflow would occur..
358+
*/
359+
int checkExecArg0BufferOverflow(char *const *argv,
360+
const char *executablePath, const char *processedExecutablePath,
361+
bool interpreterSet);
362+
279363
#ifdef __cplusplus
280364
}
281365
#endif

‎lib/termux-exec_nos_c_tre/src/termux/api/termux_exec/ld_preload/direct/exec/ExecIntercept.c‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,14 @@ int execveInterceptInternal(const char *origExecutablePath, char *const argv[],
295295

296296

297297

298+
#if defined LIBTERMUX_EXEC__NOS__C__EXECVE_CALL__CHECK_ARGV0_BUFFER_OVERFLOW && LIBTERMUX_EXEC__NOS__C__EXECVE_CALL__CHECK_ARGV0_BUFFER_OVERFLOW == 1
299+
if (checkExecArg0BufferOverflow(argv, executablePath, processedExecutablePath, interpreterSet) != 0) {
300+
return -1;
301+
}
302+
#endif
303+
304+
305+
298306
if (debugLoggingEnabled) {
299307
logErrorVerbose(LOG_TAG, "Calling syscall execve");
300308
logErrorVerbose(LOG_TAG, "executable = '%s'", executablePath);
@@ -654,3 +662,45 @@ int modifyExecArgs(char *const *argv, const char ***newArgvPointer,
654662

655663
return 0;
656664
}
665+
666+
667+
668+
int checkExecArg0BufferOverflow(char *const *argv,
669+
const char *executablePath, const char *processedExecutablePath,
670+
bool interpreterSet) {
671+
logErrorVVerbose(LOG_TAG, "Checking argv[0] buffer overflow");
672+
673+
size_t argv0Length = strlen(argv[0]);
674+
if (argv0Length >= 128) {
675+
int androidBuildVersionSdk = android_buildVersionSdk_get();
676+
if (androidBuildVersionSdk < 23) {
677+
bool shouldAbort = false;
678+
char* label = "";
679+
if (interpreterSet) {
680+
char interpreterHeader[TERMUX__FILE_HEADER__BUFFER_SIZE];
681+
ssize_t interpreterHeaderLength = readFileHeader("interpreter", executablePath, interpreterHeader, sizeof(interpreterHeader));
682+
if (interpreterHeaderLength < 0) {
683+
return interpreterHeaderLength;
684+
}
685+
686+
if (isElfFile(interpreterHeader, interpreterHeaderLength)) {
687+
shouldAbort = true;
688+
label = "interpreted";
689+
}
690+
} else {
691+
// Is elf.
692+
shouldAbort = true;
693+
label = "executable";
694+
}
695+
696+
if (shouldAbort) {
697+
logStrerrorDebug(LOG_TAG, "Cannot execute %s file '%s' as argv[0] '%s' length '%zu' is '>= 128' while running on Android SDK %d",
698+
label, processedExecutablePath, argv[0], argv0Length, androidBuildVersionSdk);
699+
errno = ENAMETOOLONG;
700+
return -1;
701+
}
702+
}
703+
}
704+
705+
return 0;
706+
}

‎lib/termux-exec_nos_c_tre/tests/libtermux-exec_nos_c_tre_tests.in‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,11 +70,16 @@ libtermux_exec__nos__c__unit_binary_tests__run_command() {
7070
termux_exec__tests__log 1 "Running 'libtermux-exec_nos_c_tre_${unit_binary_tests_variant}'"
7171

7272
output="$(
73+
# cd first and execute with relative path to shorten `argv[0]`,
74+
# otherwise command will fail with exit code `1` on Android `< 6`
75+
# without any error if `argv[0]` length is `>= 128`.
76+
# Check `checkExecArg0BufferOverflow()` function in `ExecIntercept.h`.
77+
cd "$TERMUX_EXEC__TESTS__TESTS_PATH/lib/termux-exec_nos_c_tre/bin" || exit $?
7378
printf -v "$TERMUX_EXEC__TESTS__LOG_LEVEL___N" "%s" "$TERMUX_EXEC__TESTS__LOG_LEVEL" || exit $?
7479
export "${TERMUX_EXEC__TESTS__LOG_LEVEL___N?}" || exit $?
7580
export LD_PRELOAD="$TERMUX_EXEC__TESTS__PRIMARY_LD_PRELOAD_FILE_PATH" || exit $?
7681
ASAN_OPTIONS="fast_unwind_on_malloc=false:detect_leaks=$TERMUX_EXEC__TESTS__DETECT_LEAKS" LSAN_OPTIONS="report_objects=$TERMUX_EXEC__TESTS__DETECT_LEAKS" \
77-
"$TERMUX_EXEC__TESTS__TESTS_PATH/lib/termux-exec_nos_c_tre/bin/libtermux-exec_nos_c_tre_${unit_binary_tests_variant}" 2>&1)"
82+
"./libtermux-exec_nos_c_tre_${unit_binary_tests_variant}" 2>&1)"
7883
return_value=$?
7984
if [ $return_value -eq 0 ] ||
8085
{ [ $return_value -eq 141 ] &&
@@ -144,10 +149,15 @@ libtermux_exec__nos__c__runtime_binary_tests__run_command() {
144149

145150
termux_exec__tests__log 1 "Running 'libtermux-exec_nos_c_tre_${runtime_binary_tests_variant}'"
146151
(
152+
# cd first and execute with relative path to shorten `argv[0]`,
153+
# otherwise command will fail with exit code `1` on Android `< 6`
154+
# without any error if `argv[0]` length is `>= 128`.
155+
# Check `checkExecArg0BufferOverflow()` function in `ExecIntercept.h`.
156+
cd "$TERMUX_EXEC__TESTS__TESTS_PATH/lib/termux-exec_nos_c_tre/bin" || exit $?
147157
printf -v "$TERMUX_EXEC__TESTS__LOG_LEVEL___N" "%s" "$TERMUX_EXEC__TESTS__LOG_LEVEL" || exit $?
148158
export "${TERMUX_EXEC__TESTS__LOG_LEVEL___N?}" || exit $?
149159
ASAN_OPTIONS="fast_unwind_on_malloc=false:detect_leaks=$TERMUX_EXEC__TESTS__DETECT_LEAKS" LSAN_OPTIONS="report_objects=$TERMUX_EXEC__TESTS__DETECT_LEAKS" \
150-
"$TERMUX_EXEC__TESTS__TESTS_PATH/lib/termux-exec_nos_c_tre/bin/libtermux-exec_nos_c_tre_${runtime_binary_tests_variant}"
160+
"./libtermux-exec_nos_c_tre_${runtime_binary_tests_variant}"
151161
)
152162
return_value=$?
153163
if [ $return_value -ne 0 ]; then

0 commit comments

Comments
 (0)