https://bugs.kde.org/show_bug.cgi?id=399087
--- Comment #7 from John Reiser <jrei...@bitwagon.com> ---
The root cause is a symlink vulnerability! coregrind fails to do the right
thing when the target executes
int fd_i_am = open("/proc/self/exe", O_RDONLY);
upx uses mmap(,,,,fd_i_am,) to replicate portions of the address space of the
target. Because coregrind does not virtualize correctly, then upx
unfortunately replicates portions of the valgrind tool (memcheck-arm-linux)
instead.
The simplistic workaround is for coregrind to replace any syscall argument of
"/proc/self/exe" with path_to_target. (The complete list of affected syscalls
is in the source for 'strace' in its handling of the command-line sequence "-e
trace=file".) This workaround fails to recognize "/proc/PID/exe" where PID is
the decimal representation of the numeric process ID. The workaround also can
fail if the target executes chdir() or chroot().
More generally (and for "defense in depth"), coregrind should protect against
any occurrence of "/proc/self/exe" (or equivalent) during pathname resolution
by the operating system. At initialization then coregrind should [error
checking omitted!]
int fd_tool = open("/proc/self/exe", O_RDONLY); /* yes, the tool itself */
struct stat stat_tool;
fstat(fd_tool, &stat_tool);
int fd_target = open(path_to_target, O_RDONLY);
struct stat stat_target;
fstat(fd_target, &stat_target);
For any successful fd = open() by the target, then coregrind should
struct stat stat_fd;
fstat(fd, &stat_fd);
and compare stat_fd{.st_dev, .st_ino} against stat_tool{.st_dev, .st_ino} to
detect the need for virtualization. If so, then close(fd) and set fd =
dup(fd_target); System calls other than open() probably are less important,
but in theory deserve similar care. For instance, chmod would use
open+fstat+check{.st_dev, .st_ino}+fchmod+close.
Another approach is to rely on resolution of path_to_target into an absolute
path. On linux:
int fd = open(path_to_target, O_RDONLY);
char abs_path[PATH_MAX];
char tmp[32];
snprintf(tmp, sizeof(tmp), "/proc/self/fd/%d", fd);
int n = readlink(tmp, abs_path, -1+ sizeof(abs_path));
abs_path[n] = '\0'; /* ERROR CHECKING OMITTTED */
I consider the {.st_dev, .st_ino} technique to be better.
--
You are receiving this mail because:
You are watching all bug changes.
