getdents, getdents64: fix potential out-of-bounds read issues

* file.c (sys_getdents): Check for invalid d_reclen.
Avoid reading from uninitialized memory.
(sys_getdents64): Likewise.
* tests/getdents.awk: New file.
* tests/getdents.test: New test.
* tests/Makefile.am (TESTS): Add it.
(EXTRA_DIST): Add getdents.awk.

diff --git a/file.c b/file.c
index 986f4465546103a7f4d0d134600b8ea4fcf8189a..0044429911b5d12c09c174b31c89738ae3c27901 100644 (file)
--- a/file.c
+++ b/file.c
@@ -2060,7 +2060,7 @@ sys_readdir(struct tcb *tcp)
 int
 sys_getdents(struct tcb *tcp)
 {
-       int i, len, dents = 0;
+       unsigned int i, len, dents = 0;
        char *buf;
 
        if (entering(tcp)) {
@@ -2072,38 +2072,55 @@ sys_getdents(struct tcb *tcp)
                tprintf("%#lx, %lu", tcp->u_arg[1], tcp->u_arg[2]);
                return 0;
        }
-       len = tcp->u_rval;
-       /* Beware of insanely large or negative values in tcp->u_rval */
+
+       /* Beware of insanely large or too small values in tcp->u_rval */
        if (tcp->u_rval > 1024*1024)
                len = 1024*1024;
-       if (tcp->u_rval < 0)
+       else if (tcp->u_rval < (int) sizeof(struct kernel_dirent))
                len = 0;
-       buf = len ? malloc(len) : NULL;
-       if (len && !buf)
-               die_out_of_memory();
-       if (umoven(tcp, tcp->u_arg[1], len, buf) < 0) {
-               tprintf("%#lx, %lu", tcp->u_arg[1], tcp->u_arg[2]);
-               free(buf);
-               return 0;
+       else
+               len = tcp->u_rval;
+
+       if (len) {
+               buf = malloc(len);
+               if (!buf)
+                       die_out_of_memory();
+               if (umoven(tcp, tcp->u_arg[1], len, buf) < 0) {
+                       tprintf("%#lx, %lu", tcp->u_arg[1], tcp->u_arg[2]);
+                       free(buf);
+                       return 0;
+               }
+       } else {
+               buf = NULL;
        }
+
        if (!abbrev(tcp))
                tprints("{");
-       for (i = 0; i < len;) {
+       for (i = 0; len && i <= len - sizeof(struct kernel_dirent); ) {
                struct kernel_dirent *d = (struct kernel_dirent *) &buf[i];
+
                if (!abbrev(tcp)) {
+                       int oob = d->d_reclen < sizeof(struct kernel_dirent) ||
+                                 i + d->d_reclen - 1 >= len;
+                       int d_name_len = oob ? len - i : d->d_reclen;
+                       d_name_len -= offsetof(struct kernel_dirent, d_name) + 1;
+
                        tprintf("%s{d_ino=%lu, d_off=%lu, ",
                                i ? " " : "", d->d_ino, d->d_off);
-                       tprintf("d_reclen=%u, d_name=\"%s\", d_type=",
-                               d->d_reclen, d->d_name);
-                       printxval(direnttypes, buf[i + d->d_reclen - 1], "DT_???");
+                       tprintf("d_reclen=%u, d_name=\"%.*s\", d_type=",
+                               d->d_reclen, d_name_len, d->d_name);
+                       if (oob)
+                               tprints("?");
+                       else
+                               printxval(direnttypes, buf[i + d->d_reclen - 1], "DT_???");
                        tprints("}");
                }
-               if (!d->d_reclen) {
-                       tprints("/* d_reclen == 0, problem here */");
+               dents++;
+               if (d->d_reclen < sizeof(struct kernel_dirent)) {
+                       tprints("/* d_reclen < sizeof(struct kernel_dirent) */");
                        break;
                }
                i += d->d_reclen;
-               dents++;
        }
        if (!abbrev(tcp))
                tprints("}");
@@ -2117,7 +2134,10 @@ sys_getdents(struct tcb *tcp)
 int
 sys_getdents64(struct tcb *tcp)
 {
-       int i, len, dents = 0;
+       /* the minimum size of a valid dirent64 structure */
+       const unsigned int d_name_offset = offsetof(struct dirent64, d_name);
+
+       unsigned int i, len, dents = 0;
        char *buf;
 
        if (entering(tcp)) {
@@ -2130,38 +2150,52 @@ sys_getdents64(struct tcb *tcp)
                return 0;
        }
 
-       len = tcp->u_rval;
-       /* Beware of insanely large or negative tcp->u_rval */
+       /* Beware of insanely large or too small tcp->u_rval */
        if (tcp->u_rval > 1024*1024)
                len = 1024*1024;
-       if (tcp->u_rval < 0)
+       else if (tcp->u_rval < (int) d_name_offset)
                len = 0;
-       buf = len ? malloc(len) : NULL;
-       if (len && !buf)
-               die_out_of_memory();
-
-       if (umoven(tcp, tcp->u_arg[1], len, buf) < 0) {
-               tprintf("%#lx, %lu", tcp->u_arg[1], tcp->u_arg[2]);
-               free(buf);
-               return 0;
+       else
+               len = tcp->u_rval;
+
+       if (len) {
+               buf = malloc(len);
+               if (!buf)
+                       die_out_of_memory();
+               if (umoven(tcp, tcp->u_arg[1], len, buf) < 0) {
+                       tprintf("%#lx, %lu", tcp->u_arg[1], tcp->u_arg[2]);
+                       free(buf);
+                       return 0;
+               }
+       } else {
+               buf = NULL;
        }
+
        if (!abbrev(tcp))
                tprints("{");
-       for (i = 0; i < len;) {
+       for (i = 0; len && i <= len - d_name_offset; ) {
                struct dirent64 *d = (struct dirent64 *) &buf[i];
                if (!abbrev(tcp)) {
-                       tprintf("%s{d_ino=%" PRIu64 ", d_off=%" PRId64 ", ",
+                       int d_name_len;
+                       if (d->d_reclen >= d_name_offset
+                           && i + d->d_reclen <= len) {
+                               d_name_len = d->d_reclen - d_name_offset;
+                       } else {
+                               d_name_len = len - i - d_name_offset;
+                       }
+
+                       tprintf("%s{d_ino=%" PRIu64 ", d_off=%" PRId64
+                               ", d_reclen=%u, d_type=",
                                i ? " " : "",
                                d->d_ino,
-                               d->d_off);
-                       tprints("d_type=");
+                               d->d_off,
+                               d->d_reclen);
                        printxval(direnttypes, d->d_type, "DT_???");
-                       tprints(", ");
-                       tprintf("d_reclen=%u, d_name=\"%s\"}",
-                               d->d_reclen, d->d_name);
+                       tprintf(", d_name=\"%.*s\"}",
+                               d_name_len, d->d_name);
                }
-               if (!d->d_reclen) {
-                       tprints("/* d_reclen == 0, problem here */");
+               if (d->d_reclen < d_name_offset) {
+                       tprints("/* d_reclen < offsetof(struct dirent64, d_name) */");
                        break;
                }
                i += d->d_reclen;

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 922452abfd39db53b609593eb381bc857afe788c..843fa1d491a7e58cd9d271b1b65cfa0a06e164ec 100644 (file)
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -18,6 +18,7 @@ TESTS = \
        ptrace_setoptions.test \
        strace-f.test \
        qual_syscall.test \
+       getdents.test \
        scm_rights-fd.test \
        sigaction.test \
        stat.test \
@@ -34,6 +35,6 @@ net-fd.log: net.log
 
 TEST_LOG_COMPILER = $(srcdir)/run.sh
 
-EXTRA_DIST = init.sh run.sh sigaction.awk $(TESTS)
+EXTRA_DIST = init.sh run.sh getdents.awk sigaction.awk $(TESTS)
 
 CLEANFILES = $(TESTS:=.tmp)

diff --git a/tests/getdents.awk b/tests/getdents.awk
new file mode 100644 (file)
index 0000000..9e7b968
--- /dev/null
+++ b/tests/getdents.awk
@@ -0,0 +1,45 @@
+BEGIN {
+  lines = 3
+  fail = 0
+
+  i = "[0-9]+"
+  len = "[1-9]" i
+
+  d_ino = "d_ino=" i
+  d_off = "d_off=" i
+  d_reclen = "d_reclen=" len
+  d_name1 = "d_name=\"\\.\""
+  d_name2 = "d_name=\"\\.\\.\""
+  d_type = "d_type=DT_DIR"
+
+  d1_1 = "{" d_ino ", " d_off ", " d_reclen ", " d_name1 ", " d_type "}"
+  d1_2 = "{" d_ino ", " d_off ", " d_reclen ", " d_name2 ", " d_type "}"
+  d2_1 = "{" d_ino ", " d_off ", " d_reclen ", " d_type ", " d_name1 "}"
+  d2_2 = "{" d_ino ", " d_off ", " d_reclen ", " d_type ", " d_name2 "}"
+
+  getdents   =   "^getdents\\(" i ", {(" d1_1 " " d1_2 "|" d1_2 " " d1_1 ")}, " len "\\) += " len "$"
+  getdents64 = "^getdents64\\(" i ", {(" d2_1 " " d2_2 "|" d2_2 " " d2_1 ")}, " len "\\) += " len "$"
+}
+
+NR == 1 {if (match($0, getdents) || match($0, getdents64)) next}
+
+NR == 2 && /^getdents(64)?\([0-9]+, \{\}, [1-9][0-9]+\) += 0$/ {next}
+
+NR == lines && /^\+\+\+ exited with 0 \+\+\+$/ {next}
+
+{
+  print "Line " NR " does not match: " $0
+  fail=1
+}
+
+END {
+  if (NR != lines) {
+    print "Expected " lines " lines, found " NR " line(s)."
+    print ""
+    exit 1
+  }
+  if (fail) {
+    print ""
+    exit 1
+  }
+}

diff --git a/tests/getdents.test b/tests/getdents.test
new file mode 100755 (executable)
index 0000000..7fb81b8
--- /dev/null
+++ b/tests/getdents.test
@@ -0,0 +1,28 @@
+#!/bin/sh
+
+# Check that getdents/getdents64 syscalls are traced properly.
+
+. "${srcdir=.}/init.sh"
+
+check_prog grep
+check_prog ls
+check_prog mkdir
+check_prog rmdir
+
+mkdir emptydir ||
+       framework_skip_ 'failed to create an empty directory'
+
+ls emptydir ||
+       { rmdir emptydir; framework_skip_ 'failed to list an empty directory'; }
+
+args='-vegetdents,getdents64'
+$STRACE $args -o $LOG ls emptydir
+rc=$?
+rmdir emptydir
+[ $rc -eq 0 ] ||
+       { cat $LOG; fail_ "strace $args failed"; }
+
+awk -f "$srcdir"/getdents.awk $LOG ||
+       { cat $LOG; fail_ "strace $args failed to trace getdents/getdents64 properly"; }
+
+exit 0