a4b143
From 29328cbda5daea22f02e4ca94ca75783f22efc61 Mon Sep 17 00:00:00 2001
a4b143
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
a4b143
Date: Sun, 29 Sep 2013 14:40:58 +0200
a4b143
Subject: [PATCH] Fix buffer overrun when enumerating files
a4b143
a4b143
https://bugs.freedesktop.org/show_bug.cgi?id=69887
a4b143
a4b143
Based-on-a-patch-by: Hans Petter Jansson <hpj@copyleft.no>
a4b143
---
a4b143
 src/shared/util.c    | 79 +++++++++++++++++-----------------------------------
a4b143
 src/test/test-util.c | 10 +++++++
a4b143
 2 files changed, 36 insertions(+), 53 deletions(-)
a4b143
a4b143
diff --git a/src/shared/util.c b/src/shared/util.c
a4b143
index 766957a..4f80cc8 100644
a4b143
--- a/src/shared/util.c
a4b143
+++ b/src/shared/util.c
a4b143
@@ -4412,38 +4412,31 @@ int dirent_ensure_type(DIR *d, struct dirent *de) {
a4b143
 }
a4b143
 
a4b143
 int in_search_path(const char *path, char **search) {
a4b143
-        char **i, *parent;
a4b143
+        char **i;
a4b143
+        _cleanup_free_ char *parent = NULL;
a4b143
         int r;
a4b143
 
a4b143
         r = path_get_parent(path, &parent);
a4b143
         if (r < 0)
a4b143
                 return r;
a4b143
 
a4b143
-        r = 0;
a4b143
-
a4b143
-        STRV_FOREACH(i, search) {
a4b143
-                if (path_equal(parent, *i)) {
a4b143
-                        r = 1;
a4b143
-                        break;
a4b143
-                }
a4b143
-        }
a4b143
-
a4b143
-        free(parent);
a4b143
+        STRV_FOREACH(i, search)
a4b143
+                if (path_equal(parent, *i))
a4b143
+                        return 1;
a4b143
 
a4b143
-        return r;
a4b143
+        return 0;
a4b143
 }
a4b143
 
a4b143
 int get_files_in_directory(const char *path, char ***list) {
a4b143
-        DIR *d;
a4b143
-        int r = 0;
a4b143
-        unsigned n = 0;
a4b143
-        char **l = NULL;
a4b143
+        _cleanup_closedir_ DIR *d = NULL;
a4b143
+        size_t bufsize = 0, n = 0;
a4b143
+        _cleanup_strv_free_ char **l = NULL;
a4b143
 
a4b143
         assert(path);
a4b143
 
a4b143
         /* Returns all files in a directory in *list, and the number
a4b143
          * of files as return value. If list is NULL returns only the
a4b143
-         * number */
a4b143
+         * number. */
a4b143
 
a4b143
         d = opendir(path);
a4b143
         if (!d)
a4b143
@@ -4455,11 +4448,9 @@ int get_files_in_directory(const char *path, char ***list) {
a4b143
                 int k;
a4b143
 
a4b143
                 k = readdir_r(d, &buf.de, &de);
a4b143
-                if (k != 0) {
a4b143
-                        r = -k;
a4b143
-                        goto finish;
a4b143
-                }
a4b143
-
a4b143
+                assert(k >= 0);
a4b143
+                if (k > 0)
a4b143
+                        return -k;
a4b143
                 if (!de)
a4b143
                         break;
a4b143
 
a4b143
@@ -4469,43 +4460,25 @@ int get_files_in_directory(const char *path, char ***list) {
a4b143
                         continue;
a4b143
 
a4b143
                 if (list) {
a4b143
-                        if ((unsigned) r >= n) {
a4b143
-                                char **t;
a4b143
-
a4b143
-                                n = MAX(16, 2*r);
a4b143
-                                t = realloc(l, sizeof(char*) * n);
a4b143
-                                if (!t) {
a4b143
-                                        r = -ENOMEM;
a4b143
-                                        goto finish;
a4b143
-                                }
a4b143
-
a4b143
-                                l = t;
a4b143
-                        }
a4b143
-
a4b143
-                        assert((unsigned) r < n);
a4b143
+                        /* one extra slot is needed for the terminating NULL */
a4b143
+                        if (!GREEDY_REALLOC(l, bufsize, n + 2))
a4b143
+                                return -ENOMEM;
a4b143
 
a4b143
-                        l[r] = strdup(de->d_name);
a4b143
-                        if (!l[r]) {
a4b143
-                                r = -ENOMEM;
a4b143
-                                goto finish;
a4b143
-                        }
a4b143
+                        l[n] = strdup(de->d_name);
a4b143
+                        if (!l[n])
a4b143
+                                return -ENOMEM;
a4b143
 
a4b143
-                        l[++r] = NULL;
a4b143
+                        l[++n] = NULL;
a4b143
                 } else
a4b143
-                        r++;
a4b143
+                        n++;
a4b143
         }
a4b143
 
a4b143
-finish:
a4b143
-        if (d)
a4b143
-                closedir(d);
a4b143
-
a4b143
-        if (r >= 0) {
a4b143
-                if (list)
a4b143
-                        *list = l;
a4b143
-        } else
a4b143
-                strv_free(l);
a4b143
+        if (list) {
a4b143
+                *list = l;
a4b143
+                l = NULL; /* avoid freeing */
a4b143
+        }
a4b143
 
a4b143
-        return r;
a4b143
+        return n;
a4b143
 }
a4b143
 
a4b143
 char *strjoin(const char *x, ...) {
a4b143
diff --git a/src/test/test-util.c b/src/test/test-util.c
a4b143
index ad13d53..c5762ed 100644
a4b143
--- a/src/test/test-util.c
a4b143
+++ b/src/test/test-util.c
a4b143
@@ -27,6 +27,7 @@
a4b143
 #include <errno.h>
a4b143
 
a4b143
 #include "util.h"
a4b143
+#include "strv.h"
a4b143
 
a4b143
 static void test_streq_ptr(void) {
a4b143
         assert_se(streq_ptr(NULL, NULL));
a4b143
@@ -582,6 +583,14 @@ static void test_fstab_node_to_udev_node(void) {
a4b143
         free(n);
a4b143
 }
a4b143
 
a4b143
+static void test_get_files_in_directory(void) {
a4b143
+        _cleanup_strv_free_ char **l = NULL, **t = NULL;
a4b143
+
a4b143
+        assert_se(get_files_in_directory("/tmp", &l) >= 0);
a4b143
+        assert_se(get_files_in_directory(".", &l) >= 0);
a4b143
+        assert_se(get_files_in_directory(".", NULL) >= 0);
a4b143
+}
a4b143
+
a4b143
 int main(int argc, char *argv[]) {
a4b143
         test_streq_ptr();
a4b143
         test_first_word();
a4b143
@@ -618,6 +627,7 @@ int main(int argc, char *argv[]) {
a4b143
         test_parse_user_at_host();
a4b143
         test_split_pair();
a4b143
         test_fstab_node_to_udev_node();
a4b143
+        test_get_files_in_directory();
a4b143
 
a4b143
         return 0;
a4b143
 }