Warn for execve syscall with argv or argv[0] being NULL.

For execve valgrind would silently fail when argv was NULL or
unadressable. Make sure that this produces a warning under memcheck.

The linux kernel accepts argv[0] being NULL, but most other kernels
don't since posix says it should be non-NULL and it causes argc to
be zero which is unexpected and might cause security issues.

This adjusts some testcases so they don't rely on execve succeeding
when argv is NULL and expect warnings about argv or argv[0] being
NULL or unaddressable.

https://bugs.kde.org/show_bug.cgi?id=450437
This commit is contained in:
Mark Wielaard 2022-02-16 22:56:31 +01:00
parent f540c79937
commit 8eb547054a
10 changed files with 94 additions and 27 deletions

1
NEWS
View File

@ -93,6 +93,7 @@ are not entered into bugzilla tend to get forgotten about or ignored.
449838 sigsegv liburing the 'impossible' happened for io_uring_setup
450025 Powerc: ACC file not implemented as a logical overlay of the VSR
registers.
450437 Warn for execve syscall with argv or argv[0] being NULL
450536 Powerpc: valgrind throws 'facility scv unavailable exception'
451626 Syscall param bpf(attr->raw_tracepoint.name) points to unaddressable byte(s)
451827 [ppc64le] VEX temporary storage exhausted with several vbpermq instructions

View File

@ -2933,6 +2933,7 @@ void handle_pre_sys_execve(ThreadId tid, SyscallStatus *status, Addr pathname,
Bool setuid_allowed, trace_this_child;
const char *str;
char str2[30], str3[30];
Addr arg_2_check = arg_2;
switch (execveType) {
case EXECVE:
@ -2951,15 +2952,26 @@ void handle_pre_sys_execve(ThreadId tid, SyscallStatus *status, Addr pathname,
VG_(strcpy)(str2, str);
VG_(strcpy)(str3, str);
if (arg_2 != 0) {
/* At least the terminating NULL must be addressable. */
if (!ML_(safe_to_deref)((HChar **) (Addr)arg_2, sizeof(HChar *))) {
SET_STATUS_Failure(VKI_EFAULT);
return;
VG_(strcat)(str2, "(argv)");
VG_(strcat)(str3, "(argv[0])");
/* argv[] should not be NULL and valid. */
PRE_MEM_READ(str2, arg_2_check, sizeof(Addr));
/* argv[0] should not be NULL and valid. */
if (ML_(safe_to_deref)((HChar **) (Addr)arg_2_check, sizeof(HChar *))) {
Addr argv0 = *(Addr*)arg_2_check;
PRE_MEM_RASCIIZ( str3, argv0 );
/* The rest of argv can be NULL or a valid string pointer. */
if (VG_(am_is_valid_for_client)(arg_2_check, sizeof(HChar), VKI_PROT_READ)) {
arg_2_check += sizeof(HChar*);
str3[VG_(strlen)(str)] = '\0';
VG_(strcat)(str3, "(argv[i])");
ML_(pre_argv_envp)( arg_2_check, tid, str2, str3 );
}
VG_(strcat)(str2, "(argv)");
VG_(strcat)(str3, "(argv[i])");
ML_(pre_argv_envp)( arg_2, tid, str2, str3 );
} else {
SET_STATUS_Failure(VKI_EFAULT);
return;
}
// Reset helper strings to syscall name.
str2[VG_(strlen)(str)] = '\0';

View File

@ -75,6 +75,11 @@ Syscall param execve(filename) points to unaddressable byte(s)
by 0x........: main (scalar.c:91)
Address 0x........ is not stack'd, malloc'd or (recently) free'd
Syscall param execve(argv) points to unaddressable byte(s)
...
by 0x........: main (scalar.c:91)
Address 0x........ is not stack'd, malloc'd or (recently) free'd
-----------------------------------------------------
49: __NR_chdir 1s 1m
-----------------------------------------------------
@ -576,13 +581,13 @@ Syscall param getpriority(who) contains uninitialised byte(s)
-----------------------------------------------------
140: __NR_setpriority 3s 0m
-----------------------------------------------------
More than 100 errors detected. Subsequent errors
will still be recorded, but in less detail than before.
Syscall param setpriority(which) contains uninitialised byte(s)
...
by 0x........: main (scalar.c:458)
More than 100 errors detected. Subsequent errors
will still be recorded, but in less detail than before.
Syscall param setpriority(who) contains uninitialised byte(s)
...
by 0x........: main (scalar.c:458)

View File

@ -4,7 +4,7 @@ int main(void)
{
char* null_filename = NULL;
char* bad[2] = { (char*)1, NULL };
char* good[1] = { NULL };
char* good[2] = { "true", NULL };
execve(null_filename, bad, bad);
execve("/bin/true", good, good);

View File

@ -3,7 +3,7 @@ Syscall param execve(filename) points to unaddressable byte(s)
by 0x........: main (execve1.c:9)
Address 0x........ is not stack'd, malloc'd or (recently) free'd
Syscall param execve(argv[i]) points to unaddressable byte(s)
Syscall param execve(argv[0]) points to unaddressable byte(s)
...
by 0x........: main (execve1.c:9)
Address 0x........ is not stack'd, malloc'd or (recently) free'd

View File

@ -3,3 +3,8 @@ Syscall param execve(filename) points to unaddressable byte(s)
by 0x........: main (execve2.c:9)
Address 0x........ is not stack'd, malloc'd or (recently) free'd
Syscall param execve(argv) points to unaddressable byte(s)
...
by 0x........: main (execve2.c:9)
Address 0x........ is not stack'd, malloc'd or (recently) free'd

View File

@ -17,3 +17,15 @@ Syscall param execveat(argv) points to uninitialised byte(s)
at 0x........: malloc (vg_replace_malloc.c:...)
by 0x........: main (sys-execveat.c:41)
Syscall param execveat(argv[0]) points to unaddressable byte(s)
...
by 0x........: sys_execveat (sys-execveat.c:16)
by 0x........: main (sys-execveat.c:51)
Address 0x........ is not stack'd, malloc'd or (recently) free'd
Syscall param execveat(argv) points to unaddressable byte(s)
...
by 0x........: sys_execveat (sys-execveat.c:16)
by 0x........: main (sys-execveat.c:52)
Address 0x........ is not stack'd, malloc'd or (recently) free'd

View File

@ -95,9 +95,9 @@ int main(void)
char *argv_envp[] = {(char *) (x0 + 1), NULL};
GO(__NR_execve, "4s 2m");
SY(__NR_execve, x0 + 1, x0 + argv_envp, x0); FAIL;
char *argv_ok[] = {"frob", NULL};
GO(__NR_execve, "4s 2m");
SY(__NR_execve, x0 + 1, x0, x0 + argv_envp); FAIL;
SY(__NR_execve, x0 + 1, x0 + argv_ok, x0 + argv_envp); FAIL;
// __NR_chdir 12
GO(__NR_chdir, "1s 1m");

View File

@ -170,6 +170,11 @@ Syscall param execve(filename) points to unaddressable byte(s)
by 0x........: main (scalar.c:90)
Address 0x........ is not stack'd, malloc'd or (recently) free'd
Syscall param execve(argv) points to unaddressable byte(s)
...
by 0x........: main (scalar.c:90)
Address 0x........ is not stack'd, malloc'd or (recently) free'd
-----------------------------------------------------
11: __NR_execve 3s 1m
-----------------------------------------------------
@ -190,6 +195,11 @@ Syscall param execve(filename) points to unaddressable byte(s)
by 0x........: main (scalar.c:93)
Address 0x........ is not stack'd, malloc'd or (recently) free'd
Syscall param execve(argv) points to unaddressable byte(s)
...
by 0x........: main (scalar.c:93)
Address 0x........ is not stack'd, malloc'd or (recently) free'd
-----------------------------------------------------
11: __NR_execve 4s 2m
-----------------------------------------------------
@ -216,7 +226,7 @@ Syscall param execve(argv) points to uninitialised byte(s)
Address 0x........ is on thread 1's stack
in frame #1, created by main (scalar.c:29)
Syscall param execve(argv[i]) points to unaddressable byte(s)
Syscall param execve(argv[0]) points to unaddressable byte(s)
...
by 0x........: main (scalar.c:97)
Address 0x........ is not stack'd, malloc'd or (recently) free'd
@ -564,6 +574,9 @@ Syscall param pipe(filedes) contains uninitialised byte(s)
...
by 0x........: main (scalar.c:225)
More than 100 errors detected. Subsequent errors
will still be recorded, but in less detail than before.
Syscall param pipe(filedes) points to unaddressable byte(s)
...
by 0x........: main (scalar.c:225)
@ -576,9 +589,6 @@ Syscall param times(buf) contains uninitialised byte(s)
...
by 0x........: main (scalar.c:229)
More than 100 errors detected. Subsequent errors
will still be recorded, but in less detail than before.
Syscall param times(buf) points to unaddressable byte(s)
...
by 0x........: main (scalar.c:229)

View File

@ -7,20 +7,42 @@ int main(int argc, char **argv)
if (argc == 1)
{
// This tests the case where argv and envp are NULL, which is easy to
// get wrong because it's an unusual case.
// get wrong because it's an unusual case. It is also bad and only
// "worked" by accident with the linux kernel.
char *const argv_exe[] = {"true", NULL};
char *const v_null[] = { NULL };
char *const v_minus_one[] = { (char *const) -1, NULL };
#if defined(VGO_solaris)
// Solaris requires non-NULL argv parameter
char *const argv_exe[] = {"true", NULL};
if (execve("/bin/true", argv_exe, NULL) < 0)
const char *exe = "/bin/true";
#elif defined(VGO_darwin)
if (execve("/usr/bin/true", NULL, NULL) < 0)
const char *exe = "/usr/bin/true";
#elif defined(VGO_freebsd)
char *const argv_exe[] = {"true", NULL};
if (execve("/usr/bin/true", argv_exe, NULL) < 0)
const char *exe = "/usr/bin/true";
#else
if (execve("/bin/true", NULL, NULL) < 0)
const char *exe = "/bin/true";
#endif
/* Try some bad argv and envp arguments, make sure the executable
doesn't actually exists, so execve doesn't accidentally succeeds. */
if (execve("/%/", NULL, NULL) >= 0)
printf ("WHAT?");
if (execve("/%/", (void *)-1, NULL) >= 0)
printf ("WHAT?");
if (execve("/%/", v_null, NULL) >= 0)
printf ("WHAT?");
if (execve("/%/", v_null, v_null) >= 0)
printf ("WHAT?");
if (execve("/%/", v_minus_one, NULL) >= 0)
printf ("WHAT?");
if (execve("/%/", v_minus_one, v_null) >= 0)
printf ("WHAT?");
if (execve("/%/", v_minus_one, v_minus_one) >= 0)
printf ("WHAT?");
/* Finally a correct execve. */
if (execve(exe, argv_exe, NULL) < 0)
{
perror("execve");
exit(1);