Fix execveat() with AT_FDCWD and relative path, add more checks

This update does address two closely related problems:

1) In case execveat() is called with a special file descriptor value
of AT_FDCWD (-100), it should accept this special value, and
interpret the provided pathname as relative to the current working
directory of the calling process (like execve(2)) instead of
failing with EBADF, as it does without this patch.

Covered by LTP testcase execveat01.

https://bugs.kde.org/show_bug.cgi?id=506806

2) Add checks preventing execveat() of symlinked programs in case
AT_SYMLINK_NOFOLLOW was specified.

Add checks preventing execveat() from passing in case invalid
flag was specified.

Covered by LTP testcase execveat02.

https://bugs.kde.org/show_bug.cgi?id=506813

diff --git a/NEWS b/NEWS
index 32bea935318b0694c296cf18f2eeae0ad0915413..cf879c8d20d9599f0ac4f59b74dd3b07b2f29865 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -58,6 +58,8 @@ are not entered into bugzilla tend to get forgotten about or ignored.
 506076  unimplemented fcntl command: 1028 (F_CREATED_QUERY)
 506499  Unhandled syscall 592 (exterrctl - FreeBSD
 506795  Better report which clone flags are problematic
+506806  Fix execveat() with AT_FDCWD and relative path
+506813  The execveat wrapper needs to do more checking
 506910  openat2 with RESOLVE_NO_MAGICLINKS succeeds on /proc/self/exe
 506928  Wrap (deprecated) linux specific ustat syscall
 506929  Wrap (deprecated) linux sysfs syscall

diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-linux.c
index 66e5ca62e70b0ffa0e1ffc432b899f2ed40af375..c81d941a74a4f31d2223a28641481f5d7c69cf06 100644 (file)
--- a/coregrind/m_syswrap/syswrap-linux.c
+++ b/coregrind/m_syswrap/syswrap-linux.c
@@ -13893,12 +13893,12 @@ PRE(sys_execveat)
    return;
 #endif
 
+   Int arg_1 = (Int) ARG1;
    const HChar *path = (const HChar*) ARG2;
    Addr arg_2    = ARG3;
    Addr arg_3    = ARG4;
    const HChar   *buf;
    HChar         *abs_path = NULL;
-   Bool check_at_symlink = False;
    Bool check_pathptr = True;
 
    if (ML_(safe_to_deref) (path, 1)) {
@@ -13906,8 +13906,12 @@ PRE(sys_execveat)
         * and just pass the pathname, try to determine
         * the absolute path otherwise. */
        if (path[0] != '/') {
-           /* Check dirfd is a valid fd. */
-           if (!ML_(fd_allowed)(ARG1, "execveat", tid, False)) {
+           /* Check dirfd is a valid fd.
+            * BUT: allow special value of AT_FDCWD (-101) per the execveat(2) man page:
+            *      If pathname is relative and dirfd is the special value AT_FDCWD,
+            *      then pathname is interpreted relative to the current working directory
+            *      of the calling process  (like execve(2)). */
+           if (arg_1 != VKI_AT_FDCWD && !ML_(fd_allowed)(arg_1, "execveat", tid, False)) {
                SET_STATUS_Failure( VKI_EBADF );
                return;
            }
@@ -13915,38 +13919,33 @@ PRE(sys_execveat)
               set then dirfd describes the whole path. */
            if (path[0] == '\0') {
                if (ARG5 & VKI_AT_EMPTY_PATH) {
-                   if (VG_(resolve_filename)(ARG1, &buf)) {
+                   if (VG_(resolve_filename)(arg_1, &buf)) {
                        path = buf;
                        check_pathptr = False;
                    }
                }
-           }
-           else if (ARG1 == VKI_AT_FDCWD) {
-               check_at_symlink = True;
-           } else
-               if (ARG5 & VKI_AT_SYMLINK_NOFOLLOW)
-                   check_at_symlink = True;
-               else if (VG_(resolve_filename)(ARG1, &buf)) {
+           } else if (VG_(resolve_filename)(arg_1, &buf)) {
                   abs_path = VG_(malloc)("execveat",
                                           (VG_(strlen)(buf) + 1
                                           + VG_(strlen)(path) + 1));
                    VG_(sprintf)(abs_path, "%s/%s", buf, path);
                    path = abs_path;
                    check_pathptr = False;
-               }
-               else
-                   path = NULL;
-           if (check_at_symlink) {
-               struct vg_stat statbuf;
-               SysRes statres;
-
-               statres = VG_(stat)(path, &statbuf);
-               if (sr_isError(statres) || VKI_S_ISLNK(statbuf.mode)) {
-                   SET_STATUS_Failure( VKI_ELOOP );
-                   return;
-               }
            }
        }
+       if (ARG5 & VKI_AT_SYMLINK_NOFOLLOW) {
+          struct vg_stat statbuf;
+          SysRes statres;
+          statres = VG_(stat)(path, &statbuf);
+          if (sr_isError(statres) || VKI_S_ISLNK(statbuf.mode)) {
+             SET_STATUS_Failure( VKI_ELOOP );
+             return;
+          }
+       }
+       if(ARG5 & ~(VKI_AT_SYMLINK_NOFOLLOW | VKI_AT_EMPTY_PATH)) {
+          SET_STATUS_Failure( VKI_EINVAL );
+          return;
+       }
    } else {
        SET_STATUS_Failure(VKI_EFAULT);
        return;