isa/sudo
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Avoid potentially overwriting string table when writing argv.

Browse Source 
In compat mode, if argc is odd, writing the last pointer of argv will
overlap with the address of argv[0], so leave an extra word in between.
Also remove incorrect comments about PTRACE_PEEKDATA unaligned access.
This commit is contained in:
Todd C. Miller
2022-05-06 19:46:22 -06:00
parent 0a2975367e
commit 206cd10ed5
1 changed files with 7 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
src/exec_ptrace.c
View File

@@ -320,7 +320,6 @@ ptrace_read_string(pid_t pid, long addr, char *buf, size_t bufsize)
debug_return_ssize_t(-1); debug_return_ssize_t(-1);
} }
/* XXX - this could be optimized. */
cp = (char *)&word; cp = (char *)&word;
for (i = 0; i < sizeof(long); i++) { for (i = 0; i < sizeof(long); i++) {
if (bufsize == 0) { if (bufsize == 0) {
@@ -353,7 +352,6 @@ ptrace_read_vec(pid_t pid, struct sudo_ptrace_regs *regs, long addr,
/* Fill in vector. */ /* Fill in vector. */
for (;;) { for (;;) {
/* XXX - unaligned access for compat binaries */
long word = ptrace(PTRACE_PEEKDATA, pid, addr, NULL); long word = ptrace(PTRACE_PEEKDATA, pid, addr, NULL);
word &= regs->addrmask; word &= regs->addrmask;
switch (word) { switch (word) {
@@ -393,7 +391,6 @@ ptrace_get_vec_len(pid_t pid, struct sudo_ptrace_regs *regs, long addr)
debug_decl(ptrace_get_vec_len, SUDO_DEBUG_EXEC); debug_decl(ptrace_get_vec_len, SUDO_DEBUG_EXEC);
for (;;) { for (;;) {
/* XXX - unaligned access for compat binaries */
long word = ptrace(PTRACE_PEEKDATA, pid, addr, NULL); long word = ptrace(PTRACE_PEEKDATA, pid, addr, NULL);
word &= regs->addrmask; word &= regs->addrmask;
switch (word) { switch (word) {
@@ -900,12 +897,15 @@ ptrace_intercept_execve(pid_t pid, struct intercept_closure *closure)
/* /*
* Calculate the amount of space required for pointers + strings. * Calculate the amount of space required for pointers + strings.
* We align everything on a word boundary to simplify writes, * We align everything on a word boundary to avoid overlapping
* ptrace(2) may not be able to write to unaligned addresses. * writes since ptrace(2) does everything in native word units.
*
* In compat mode, if argc is odd, writing the last pointer will
* overlap the first string so leave an extra word in between.
*/ */
if (argv_mismatch) { if (argv_mismatch) {
/* argv pointers */ /* argv pointers */
len = (argc + 1) * regs.wordsize; len = (argc + 1 + regs.compat) * regs.wordsize;
space += WORDALIGN(len); space += WORDALIGN(len);
/* argv strings */ /* argv strings */
@@ -929,12 +929,11 @@ ptrace_intercept_execve(pid_t pid, struct intercept_closure *closure)
set_sc_arg2(&regs, sp); set_sc_arg2(&regs, sp);
/* Skip over argv pointers (plus NULL) for string table. */ /* Skip over argv pointers (plus NULL) for string table. */
strtab += WORDALIGN((argc + 1) * regs.wordsize); strtab += WORDALIGN((argc + 1 + regs.compat) * regs.wordsize);
/* Copy new argv (+ NULL) into tracee one word at a time. */ /* Copy new argv (+ NULL) into tracee one word at a time. */
for (i = 0; i < argc; i++) { for (i = 0; i < argc; i++) {
/* Store string address as new argv[i]. */ /* Store string address as new argv[i]. */
/* XXX - unaligned access for compat binaries */
if (ptrace(PTRACE_POKEDATA, pid, sp, strtab) == -1) { if (ptrace(PTRACE_POKEDATA, pid, sp, strtab) == -1) {
sudo_warn("ptrace(PTRACE_POKEDATA, %d, 0x%lx, 0x%lx)", sudo_warn("ptrace(PTRACE_POKEDATA, %d, 0x%lx, 0x%lx)",
(int)pid, sp, strtab); (int)pid, sp, strtab);
@@ -948,7 +947,6 @@ ptrace_intercept_execve(pid_t pid, struct intercept_closure *closure)
goto done; goto done;
strtab += len; strtab += len;
} }
/* XXX - unaligned access for compat binaries */
if (ptrace(PTRACE_POKEDATA, pid, sp, NULL) == -1) { if (ptrace(PTRACE_POKEDATA, pid, sp, NULL) == -1) {
sudo_warn("ptrace(PTRACE_POKEDATA, %d, 0x%lx, NULL)", sudo_warn("ptrace(PTRACE_POKEDATA, %d, 0x%lx, NULL)",
(int)pid, sp); (int)pid, sp);