Skip to content

Commit 4477568

Browse files
agnostic-apolloagnostic-apollo
committed
Fixed(ExecIntercept): Fix LD_VARS not being unset for system executables if executed with fexecve() or fd path, and their file header being read instead of skipped as per 3300bfb
File descriptor executable paths are in the format `/proc/self/fd/<num>` or `/proc/<pid>/fd/<num>`, normally for `fexecve()`, but can be passed manually by callers. For such paths, checks would fail for if the path is under system directories with `isExecutableUnderSystemDir()` or if `LD_VARS` should be unset with `shouldUnsetLDVarsFromEnv()`, as path would start with `/proc` instead of one of the system directories. Now we find the real path before any checks are done by calling `readlink()` via `getRegularFileFdRealPath()` on the fd path. Note that `fexecve()` is only supported for Android `>= 9` and `CANNOT LINK EXECUTABLE` errors due to `LD_VARS` being set like `LD_LIBRARY_PATH=/data/data/com.termux/files/usr/lib` when calling system binaries only seem to occur on older Android versions. So we simulate `fexecve()` for tests by opening a system executable, generating fd path manually and then executing it. Following are examples of errors caused by `LD_LIBRARY_PATH` being set. Termux app only exports it for Android `5` and `6` though. ``` $ LD_PRELOAD= bash $ LD_LIBRARY_PATH=/data/data/com.termux/files/usr/lib PATH=/system/bin /system/bin/{am,pm} CANNOT LINK EXECUTABLE: could not load library "libandroid_runtime.so" needed by "app_process"; caused by could not load library "libhwui.so" needed by "libandroid_runtime.so"; caused by could not load library "libRS.so" needed by "libhwui.so"; caused by could not load library "libbcc.so" needed by "libRS.so"; caused by could not load library "libbcinfo.so" needed by "libbcc.so"; caused by cannot locate symbol "_ZNK4llvm6MDNode10getOperandEj" referenced by "libbcinfo.so"... $ LD_LIBRARY_PATH=/data/data/com.termux/files/usr/lib PATH=/system/bin /system/bin/dalvikvm Aborted dlopen("libjavacore.so", RTLD_LAZY) failed: dlopen failed: cannot locate symbol "bn_expand2" referenced by "libjavacore.so"... LD_LIBRARY_PATH=/data/data/com.termux/files/usr/lib PATH=/system/bin /system/bin/{am,pm} CANNOT LINK EXECUTABLE: cannot locate symbol "u_charMirror_55" referenced by "/system/lib64/libandroid_runtime.so"... $ LD_LIBRARY_PATH=/data/data/com.termux/files/usr/lib PATH=/system/bin /system/bin/dalvikvm Aborted art/runtime/runtime.cc:1149] LoadNativeLibrary failed for "libjavacore.so": dlopen failed: cannot locate symbol "_ZTVN6icu_5513UnicodeStringE" referenced by "/system/lib64/libjavacore.so"... ~ $ LD_LIBRARY_PATH=/data/data/com.termux/files/usr/lib PATH=/system/bin /system/bin/{am,pm} CANNOT LINK EXECUTABLE "app_process": cannot locate symbol "glTexGenxvOES" referenced by "/system/lib64/libandroid_runtime.so"... Aborted $ LD_LIBRARY_PATH=/data/data/com.termux/files/usr/lib PATH=/system/bin /system/bin/dalvikvm Failed to initialize JNI invocation API from (null) Failed to dlopen libart.so: dlopen failed: cannot locate symbol "XzUnpacker_Construct" referenced by "/system/lib64/libunwind.so"... ```
1 parent adf08af commit 4477568

File tree

2 files changed

+171
-1
lines changed

2 files changed

+171
-1
lines changed

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

+40-1
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,12 @@ int execveInterceptInternal(const char *origExecutablePath, char *const argv[],
9191
// For instance, `$TERMUX__PREFIX/bin/ls` is a symlink to `$TERMUX__PREFIX/bin/coreutils`,
9292
// but we need to execute `$TERMUX__PREFIX/bin/ls` `/system/bin/linker $TERMUX__PREFIX/bin/ls`
9393
// so that coreutils knows what to execute.
94+
// - For an fd path, like `/proc/self/fd/<num>` or `/proc/<pid>/fd/<num>`,
95+
// normally for the `fexecve()` call, we find its real path,
96+
// so that so if its not under a system directory. then its file
97+
// header is always read. And if it is under a system directory,
98+
// then `LD_VARS_TO_UNSET` are unset properly as per
99+
// `unsetLdVarsFromEnv` or `unsetLdPreloadFromEnv`.
94100
// - For an absolute path, we need to normalize first so that an
95101
// unnormalized prefix like `/usr/./bin` is replaced with `/usr/bin`
96102
// so that `termuxPrefixPath()` can successfully match it to
@@ -117,7 +123,24 @@ int execveInterceptInternal(const char *origExecutablePath, char *const argv[],
117123

118124

119125
char processedExecutablePathBuffer[PATH_MAX];
120-
if (executablePath[0] == '/') {
126+
if (isFdPath(executablePath)) {
127+
executablePath = getRegularFileFdRealPath(LOG_TAG, executablePath,
128+
processedExecutablePathBuffer, sizeof(processedExecutablePathBuffer));
129+
if (executablePath == NULL) {
130+
// `execve()` is expected to return `EACCES` for
131+
// non-regular files and is also mentioned in its man page.
132+
// > EACCES The file or a script interpreter is not a regular file.
133+
// The `getRegularFileFdRealPath()` function will return
134+
// following `errno` for non-regular files.
135+
if (errno == EISDIR || errno == ENXIO) {
136+
errno = EACCES;
137+
}
138+
logStrerrorDebug(LOG_TAG, "Failed to get real path for fd executable path '%s'", origExecutablePath);
139+
return -1;
140+
}
141+
142+
logErrorVVerbose(LOG_TAG, "real_executable: '%s'", executablePath);
143+
} else if (executablePath[0] == '/') {
121144
// If path is absolute, then normalize first and then replace termux prefix.
122145
executablePath = normalizePath(executablePathBuffer, false, true);
123146
if (executablePath == NULL) {
@@ -210,6 +233,22 @@ int execveInterceptInternal(const char *origExecutablePath, char *const argv[],
210233
// - https://github.com/termux/termux-exec-package/issues/32
211234
// - `/system/bin/app_process`.
212235
// - https://github.com/termux/termux-app/issues/4440#issuecomment-2746002438
236+
//
237+
// A script running with `system_linker_exec` must always be run
238+
// with its interpreter in the shebang instead of directly,
239+
// otherwise execution will fail with errors like following.
240+
// The `system_linker_exec` is only used if path is under
241+
// Termux app data directory, and we skip reading file header only
242+
// if its under a system directory. Since app data directories
243+
// are either under `/data` or `/mnt`, there is no conflict with
244+
// system directories, and so file headers for files under app
245+
// data directories should always be read.
246+
// error: "/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-linux-script.sh" \
247+
// is too small to be an ELF executable: only found 52 bytes
248+
// - https://cs.android.com/android/platform/superproject/+/android-14.0.0_r1:bionic/linker/linker_phdr.cpp;l=216
249+
// error: "/data/data/com.termux/files/usr/bin/login" \
250+
// has bad ELF magic: 23212f64
251+
// - https://cs.android.com/android/platform/superproject/+/android-14.0.0_r1:bionic/linker/linker_phdr.cpp;l=234
213252
if (isExecutableUnderSystemDir(executablePath)) {
214253
logErrorVVerbose(LOG_TAG, "read_file_header: '0'");
215254
} else {

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

+131
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,7 @@ FEXECVE_CALL_IMPL()
275275

276276

277277
void test__execIntercept__Basic();
278+
void test__execIntercept__AndroidTools(char* envCurrentPath);
278279
void test__execIntercept__Files(const char* termuxExec_tests_testsPath, const char* currentPath, char* envCurrentPath);
279280
void test__execIntercept__PackageManager();
280281

@@ -317,6 +318,7 @@ void test__execIntercept() {
317318
// - https://cs.android.com/android/platform/superproject/+/android-14.0.0_r18:bionic/tests/utils.h;l=200
318319

319320
test__execIntercept__Basic();
321+
test__execIntercept__AndroidTools(envCurrentPath);
320322
test__execIntercept__Files(termuxExec_tests_testsPath, currentPath, envCurrentPath);
321323
test__execIntercept__PackageManager();
322324

@@ -328,6 +330,14 @@ void test__execIntercept() {
328330

329331

330332

333+
static void test__execIntercept_initChildWithRedirectedFd(ForkInfo *info) {
334+
initChild(info);
335+
336+
info->redirectChildStdinToDevNull = true;
337+
}
338+
339+
340+
331341
void test__execIntercept__Basic() {
332342
logVVerbose(LOG_TAG, "test__execIntercept__Basic()");
333343

@@ -350,6 +360,127 @@ void test__execIntercept__Basic() {
350360
NULL);
351361
}
352362

363+
void test__execIntercept__AndroidTools(char* envCurrentPath) {
364+
logVVerbose(LOG_TAG, "test__execIntercept__AndroidTools()");
365+
366+
onExecTestChildFork = test__execIntercept_initChildWithRedirectedFd;
367+
368+
// execlp(), execvp() and execvpe() search for file to be executed in $PATH,
369+
// so set it to `/system/bin` directory.
370+
char* envNewPath = NULL;
371+
372+
if (asprintf(&envNewPath, "%s%s", ENV_PREFIX__PATH, "/apex/com.android.art/bin:/system/bin") == -1) {
373+
errno = ENOMEM;
374+
logStrerrorDebug(LOG_TAG, "asprintf failed for new '%s%s'", ENV_PREFIX__PATH, "/apex/com.android.art/bin:/system/bin");
375+
exit(1);
376+
}
377+
378+
putenv(envNewPath);
379+
380+
381+
// On Android `14`, `/system/bin/am` can only be run with `adb`
382+
// and `root` users and will normally have no output and exit code
383+
// will be `255`.
384+
// - https://github.com/termux/TermuxAm/commit/8a08e9bd
385+
if (android_buildVersionSdk_get() < 34) {
386+
char* amPath = "/system/bin/am";
387+
388+
runAllExecWrappersTest("android-am",
389+
0, 0, 0, 0,
390+
-1, "^.*<INTENT> specifications.*", -1, "^.*<INTENT> specifications.*", REG_EXTENDED | REG_ICASE,
391+
"am", amPath, environ,
392+
NULL);
393+
394+
395+
// Simulate `fexecve()`, especially for Android `< 9`.
396+
// This will also test `LD_LIBRARY_PATH` being unset on older
397+
// Android versions to prevent `CANNOT LINK EXECUTABLE` errors.
398+
int amFd = open(amPath, 0);
399+
if (amFd == -1) {
400+
logStrerror(LOG_TAG, "open() call failed for am at path '%s'", amPath);
401+
exit(1);
402+
}
403+
404+
char amFdPath[40];
405+
snprintf(amFdPath, sizeof(amFdPath), "/proc/self/fd/%d", amFd);
406+
407+
logVVerbose(LOG_TAG, "%s_exec()", "android-am-fd"); \
408+
runExecTest("android-am-fd",
409+
0, 0,
410+
-1, "^.*<INTENT> specifications.*", REG_EXTENDED | REG_ICASE,
411+
ExecVE, amFdPath, environ, amPath, NULL);
412+
}
413+
414+
415+
// On Android `< 9`, `/system/bin/pm` did not have a `#!/system/bin/sh`
416+
// interpreter and `execve` would fail with `Exec format error (ENOEXEC)`.
417+
// So interpret the `pm` script with `sh` instead of executing it.
418+
// - https://cs.android.com/android/_/android/platform/frameworks/base/+/6c168885
419+
char* pmPath = "/system/bin/pm";
420+
if (android_buildVersionSdk_get() < 29) {
421+
runAllExecWrappersTest("android-pm",
422+
0, 0, 0, 0,
423+
-1, "^.*path to the \\.apk.*", -1, "^.*path to the \\.apk.*", REG_EXTENDED | REG_ICASE,
424+
NULL, "/system/bin/sh", environ,
425+
pmPath, NULL);
426+
427+
} else {
428+
runAllExecWrappersTest("android-pm",
429+
0, 0, 0, 0,
430+
-1, "^.*path to the \\.apk.*", -1, "^.*path to the \\.apk.*", REG_EXTENDED | REG_ICASE,
431+
"pm", pmPath, environ,
432+
NULL);
433+
}
434+
435+
436+
// On Android `>= 13`, `dalvikvm` is part of Android Runtime APEX module.
437+
// - https://cs.android.com/android/_/android/platform/art/+/38a938e2
438+
// - https://cs.android.com/android/_/android/platform/bionic/+/6d5277db
439+
char* dalvikvmPath;
440+
if (access("/apex/com.android.art/bin/dalvikvm", X_OK) == 0) {
441+
dalvikvmPath = "/apex/com.android.art/bin/dalvikvm";
442+
} else {
443+
dalvikvmPath = "/system/bin/dalvikvm";
444+
}
445+
446+
char* dalvikvmOutputRegex = "^.*((-classpath ((classpath)|(\\{string value\\})))|(Class name required)).*";
447+
448+
// The `-h` flag does not print help on Android `7`, but Android `6` does.
449+
runAllExecWrappersTest("android-dalvikvm",
450+
0, 0, 0, 0,
451+
-1, dalvikvmOutputRegex, -1, dalvikvmOutputRegex, REG_EXTENDED | REG_ICASE,
452+
"dalvikvm", dalvikvmPath, environ,
453+
"-h", NULL);
454+
455+
456+
// Simulate `fexecve()`, especially for Android `< 9`.
457+
// This will also test `LD_LIBRARY_PATH` being unset on older
458+
// Android versions to prevent `CANNOT LINK EXECUTABLE` errors.
459+
int dalvikvmFd = open(dalvikvmPath, 0);
460+
if (dalvikvmFd == -1) {
461+
logStrerror(LOG_TAG, "open() call failed for dalvikvm at path '%s'", dalvikvmPath);
462+
exit(1);
463+
}
464+
465+
char dalvikvmFdPath[40];
466+
snprintf(dalvikvmFdPath, sizeof(dalvikvmFdPath), "/proc/self/fd/%d", dalvikvmFd);
467+
468+
logVVerbose(LOG_TAG, "%s_exec()", "android-dalvikvm-fd"); \
469+
runExecTest("android-dalvikvm-fd",
470+
0, 0,
471+
-1, dalvikvmOutputRegex, REG_EXTENDED | REG_ICASE,
472+
ExecVE, dalvikvmFdPath, environ, dalvikvmPath, "-h", NULL);
473+
474+
475+
putenv(envCurrentPath);
476+
477+
478+
free(envNewPath);
479+
480+
onExecTestChildFork = initChild;
481+
482+
}
483+
353484
void test__execIntercept__Files(const char* termuxExec_tests_testsPath, const char* currentPath, char* envCurrentPath) {
354485
logVVerbose(LOG_TAG, "test__execIntercept__Files()");
355486

0 commit comments

Comments
 (0)