Skip to content

Commit 815e542

Browse files
martinpittmartinpitt
authored
Merge pull request systemd#5809 from keszybz/glob-safe
Implement `safe_glob` that ignores "." and ".."
2 parents 5b3cc0c + d8c92e8 commit 815e542

File tree

9 files changed

+169
-53
lines changed

9 files changed

+169
-53
lines changed

‎src/basic/dirent-util.c‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,3 +75,14 @@ bool dirent_is_file_with_suffix(const struct dirent *de, const char *suffix) {
7575

7676
return endswith(de->d_name, suffix);
7777
}
78+
79+
struct dirent* readdir_no_dot(DIR *dirp) {
80+
struct dirent* d;
81+
82+
for (;;) {
83+
d = readdir(dirp);
84+
if (d && dot_or_dot_dot(d->d_name))
85+
continue;
86+
return d;
87+
}
88+
}

‎src/basic/dirent-util.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ int dirent_ensure_type(DIR *d, struct dirent *de);
3131
bool dirent_is_file(const struct dirent *de) _pure_;
3232
bool dirent_is_file_with_suffix(const struct dirent *de, const char *suffix) _pure_;
3333

34+
struct dirent* readdir_no_dot(DIR *dirp);
35+
3436
#define FOREACH_DIRENT(de, d, on_error) \
3537
for (errno = 0, de = readdir(d);; errno = 0, de = readdir(d)) \
3638
if (!de) { \

‎src/basic/glob-util.c‎

Lines changed: 40 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -17,54 +17,70 @@
1717
along with systemd; If not, see <http://www.gnu.org/licenses/>.
1818
***/
1919

20+
#include <dirent.h>
2021
#include <errno.h>
2122
#include <glob.h>
23+
#include <sys/types.h>
2224

25+
#include "dirent-util.h"
2326
#include "glob-util.h"
2427
#include "macro.h"
28+
#include "path-util.h"
2529
#include "strv.h"
2630

27-
int glob_exists(const char *path) {
28-
_cleanup_globfree_ glob_t g = {};
31+
int safe_glob(const char *path, int flags, glob_t *pglob) {
2932
int k;
3033

31-
assert(path);
34+
/* We want to set GLOB_ALTDIRFUNC ourselves, don't allow it to be set. */
35+
assert(!(flags & GLOB_ALTDIRFUNC));
36+
37+
if (!pglob->gl_closedir)
38+
pglob->gl_closedir = (void (*)(void *)) closedir;
39+
if (!pglob->gl_readdir)
40+
pglob->gl_readdir = (struct dirent *(*)(void *)) readdir_no_dot;
41+
if (!pglob->gl_opendir)
42+
pglob->gl_opendir = (void *(*)(const char *)) opendir;
43+
if (!pglob->gl_lstat)
44+
pglob->gl_lstat = lstat;
45+
if (!pglob->gl_stat)
46+
pglob->gl_stat = stat;
3247

3348
errno = 0;
34-
k = glob(path, GLOB_NOSORT|GLOB_BRACE, NULL, &g);
49+
k = glob(path, flags | GLOB_ALTDIRFUNC, NULL, pglob);
3550

3651
if (k == GLOB_NOMATCH)
37-
return 0;
52+
return -ENOENT;
3853
if (k == GLOB_NOSPACE)
3954
return -ENOMEM;
4055
if (k != 0)
4156
return errno > 0 ? -errno : -EIO;
57+
if (strv_isempty(pglob->gl_pathv))
58+
return -ENOENT;
4259

43-
return !strv_isempty(g.gl_pathv);
60+
return 0;
4461
}
4562

46-
int glob_extend(char ***strv, const char *path) {
63+
int glob_exists(const char *path) {
4764
_cleanup_globfree_ glob_t g = {};
4865
int k;
49-
char **p;
5066

51-
errno = 0;
52-
k = glob(path, GLOB_NOSORT|GLOB_BRACE, NULL, &g);
67+
assert(path);
5368

54-
if (k == GLOB_NOMATCH)
55-
return -ENOENT;
56-
if (k == GLOB_NOSPACE)
57-
return -ENOMEM;
58-
if (k != 0)
59-
return errno > 0 ? -errno : -EIO;
60-
if (strv_isempty(g.gl_pathv))
61-
return -ENOENT;
69+
k = safe_glob(path, GLOB_NOSORT|GLOB_BRACE, &g);
70+
if (k == -ENOENT)
71+
return false;
72+
if (k < 0)
73+
return k;
74+
return true;
75+
}
76+
77+
int glob_extend(char ***strv, const char *path) {
78+
_cleanup_globfree_ glob_t g = {};
79+
int k;
6280

63-
STRV_FOREACH(p, g.gl_pathv) {
64-
k = strv_extend(strv, *p);
65-
if (k < 0)
66-
return k;
67-
}
81+
k = safe_glob(path, GLOB_NOSORT|GLOB_BRACE, &g);
82+
if (k < 0)
83+
return k;
6884

69-
return 0;
85+
return strv_extend_strv(strv, g.gl_pathv, false);
7086
}

‎src/basic/glob-util.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,16 @@
1919
along with systemd; If not, see <http://www.gnu.org/licenses/>.
2020
***/
2121

22+
#include <glob.h>
2223
#include <stdbool.h>
2324
#include <string.h>
2425

2526
#include "macro.h"
2627
#include "string-util.h"
2728

29+
/* Note: this function modifies pglob to set various functions. */
30+
int safe_glob(const char *path, int flags, glob_t *pglob);
31+
2832
int glob_exists(const char *path);
2933
int glob_extend(char ***strv, const char *path);
3034

‎src/basic/rm-rf.c‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -182,18 +182,11 @@ int rm_rf(const char *path, RemoveFlags flags) {
182182
/* We refuse to clean the root file system with this
183183
* call. This is extra paranoia to never cause a really
184184
* seriously broken system. */
185-
if (path_equal(path, "/")) {
185+
if (path_equal_or_files_same(path, "/")) {
186186
log_error("Attempted to remove entire root file system, and we can't allow that.");
187187
return -EPERM;
188188
}
189189

190-
/* Another safe-check. Removing "/path/.." could easily remove entire root as well.
191-
* It's especially easy to do using globs in tmpfiles, like "/path/.*", which the glob()
192-
* function expands to both "/path/." and "/path/..".
193-
* Return -EINVAL to be consistent with rmdir("/path/."). */
194-
if (endswith(path, "/..") || endswith(path, "/../"))
195-
return -EINVAL;
196-
197190
if ((flags & (REMOVE_SUBVOLUME|REMOVE_ROOT|REMOVE_PHYSICAL)) == (REMOVE_SUBVOLUME|REMOVE_ROOT|REMOVE_PHYSICAL)) {
198191
/* Try to remove as subvolume first */
199192
r = btrfs_subvol_remove(path, BTRFS_REMOVE_RECURSIVE|BTRFS_REMOVE_QUOTA);

‎src/core/execute.c‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3231,11 +3231,10 @@ int exec_context_load_environment(Unit *unit, const ExecContext *c, char ***l) {
32313231

32323232
STRV_FOREACH(i, c->environment_files) {
32333233
char *fn;
3234-
int k;
3234+
int k, n;
32353235
bool ignore = false;
32363236
char **p;
32373237
_cleanup_globfree_ glob_t pglob = {};
3238-
int count, n;
32393238

32403239
fn = *i;
32413240

@@ -3253,23 +3252,19 @@ int exec_context_load_environment(Unit *unit, const ExecContext *c, char ***l) {
32533252
}
32543253

32553254
/* Filename supports globbing, take all matching files */
3256-
errno = 0;
3257-
if (glob(fn, 0, NULL, &pglob) != 0) {
3255+
k = safe_glob(fn, 0, &pglob);
3256+
if (k < 0) {
32583257
if (ignore)
32593258
continue;
32603259

32613260
strv_free(r);
3262-
return errno > 0 ? -errno : -EINVAL;
3261+
return k;
32633262
}
3264-
count = pglob.gl_pathc;
3265-
if (count == 0) {
3266-
if (ignore)
3267-
continue;
32683263

3269-
strv_free(r);
3270-
return -EINVAL;
3271-
}
3272-
for (n = 0; n < count; n++) {
3264+
/* When we don't match anything, -ENOENT should be returned */
3265+
assert(pglob.gl_pathc > 0);
3266+
3267+
for (n = 0; n < pglob.gl_pathc; n++) {
32733268
k = load_env_file(NULL, pglob.gl_pathv[n], NULL, &p);
32743269
if (k < 0) {
32753270
if (ignore)

‎src/test/test-glob-util.c‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,17 @@
1818
***/
1919

2020
#include <fcntl.h>
21+
#include <glob.h>
22+
#include <sys/stat.h>
2123
#include <unistd.h>
2224

2325
#include "alloc-util.h"
26+
#include "dirent-util.h"
2427
#include "fileio.h"
28+
#include "fs-util.h"
2529
#include "glob-util.h"
2630
#include "macro.h"
31+
#include "rm-rf.h"
2732

2833
static void test_glob_exists(void) {
2934
char name[] = "/tmp/test-glob_exists.XXXXXX";
@@ -43,8 +48,69 @@ static void test_glob_exists(void) {
4348
assert_se(r == 0);
4449
}
4550

51+
static void test_glob_no_dot(void) {
52+
char template[] = "/tmp/test-glob-util.XXXXXXX";
53+
const char *fn;
54+
55+
_cleanup_globfree_ glob_t g = {
56+
.gl_closedir = (void (*)(void *)) closedir,
57+
.gl_readdir = (struct dirent *(*)(void *)) readdir_no_dot,
58+
.gl_opendir = (void *(*)(const char *)) opendir,
59+
.gl_lstat = lstat,
60+
.gl_stat = stat,
61+
};
62+
63+
int r;
64+
65+
assert_se(mkdtemp(template));
66+
67+
fn = strjoina(template, "/*");
68+
r = glob(fn, GLOB_NOSORT|GLOB_BRACE|GLOB_ALTDIRFUNC, NULL, &g);
69+
assert_se(r == GLOB_NOMATCH);
70+
71+
fn = strjoina(template, "/.*");
72+
r = glob(fn, GLOB_NOSORT|GLOB_BRACE|GLOB_ALTDIRFUNC, NULL, &g);
73+
assert_se(r == GLOB_NOMATCH);
74+
75+
(void) rm_rf(template, REMOVE_ROOT|REMOVE_PHYSICAL);
76+
}
77+
78+
static void test_safe_glob(void) {
79+
char template[] = "/tmp/test-glob-util.XXXXXXX";
80+
const char *fn, *fn2, *fname;
81+
82+
_cleanup_globfree_ glob_t g = {};
83+
int r;
84+
85+
assert_se(mkdtemp(template));
86+
87+
fn = strjoina(template, "/*");
88+
r = safe_glob(fn, 0, &g);
89+
assert_se(r == -ENOENT);
90+
91+
fn2 = strjoina(template, "/.*");
92+
r = safe_glob(fn2, GLOB_NOSORT|GLOB_BRACE, &g);
93+
assert_se(r == -ENOENT);
94+
95+
fname = strjoina(template, "/.foobar");
96+
assert_se(touch(fname) == 0);
97+
98+
r = safe_glob(fn, 0, &g);
99+
assert_se(r == -ENOENT);
100+
101+
r = safe_glob(fn2, GLOB_NOSORT|GLOB_BRACE, &g);
102+
assert_se(r == 0);
103+
assert_se(g.gl_pathc == 1);
104+
assert_se(streq(g.gl_pathv[0], fname));
105+
assert_se(g.gl_pathv[1] == NULL);
106+
107+
(void) rm_rf(template, REMOVE_ROOT|REMOVE_PHYSICAL);
108+
}
109+
46110
int main(void) {
47111
test_glob_exists();
112+
test_glob_no_dot();
113+
test_safe_glob();
48114

49115
return 0;
50116
}

‎src/test/test-path-util.c‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "mount-util.h"
2828
#include "path-util.h"
2929
#include "rm-rf.h"
30+
#include "stat-util.h"
3031
#include "string-util.h"
3132
#include "strv.h"
3233
#include "util.h"
@@ -104,6 +105,38 @@ static void test_path(void) {
104105
assert_se(!path_equal_ptr(NULL, "/a"));
105106
}
106107

108+
static void test_path_equal_root(void) {
109+
/* Nail down the details of how path_equal("/", ...) works. */
110+
111+
assert_se(path_equal("/", "/"));
112+
assert_se(path_equal("/", "//"));
113+
114+
assert_se(!path_equal("/", "/./"));
115+
assert_se(!path_equal("/", "/../"));
116+
117+
assert_se(!path_equal("/", "/.../"));
118+
119+
/* Make sure that files_same works as expected. */
120+
121+
assert_se(files_same("/", "/") > 0);
122+
assert_se(files_same("/", "//") > 0);
123+
124+
assert_se(files_same("/", "/./") > 0);
125+
assert_se(files_same("/", "/../") > 0);
126+
127+
assert_se(files_same("/", "/.../") == -ENOENT);
128+
129+
/* The same for path_equal_or_files_same. */
130+
131+
assert_se(path_equal_or_files_same("/", "/"));
132+
assert_se(path_equal_or_files_same("/", "//"));
133+
134+
assert_se(path_equal_or_files_same("/", "/./"));
135+
assert_se(path_equal_or_files_same("/", "/../"));
136+
137+
assert_se(!path_equal_or_files_same("/", "/.../"));
138+
}
139+
107140
static void test_find_binary(const char *self) {
108141
char *p;
109142

@@ -551,6 +584,7 @@ int main(int argc, char **argv) {
551584
log_open();
552585

553586
test_path();
587+
test_path_equal_root();
554588
test_find_binary(argv[0]);
555589
test_prefixes();
556590
test_path_join();

‎src/tmpfiles/tmpfiles.c‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1093,19 +1093,14 @@ static int item_do_children(Item *i, const char *path, action_t action) {
10931093

10941094
static int glob_item(Item *i, action_t action, bool recursive) {
10951095
_cleanup_globfree_ glob_t g = {
1096-
.gl_closedir = (void (*)(void *)) closedir,
1097-
.gl_readdir = (struct dirent *(*)(void *)) readdir,
10981096
.gl_opendir = (void *(*)(const char *)) opendir_nomod,
1099-
.gl_lstat = lstat,
1100-
.gl_stat = stat,
11011097
};
11021098
int r = 0, k;
11031099
char **fn;
11041100

1105-
errno = 0;
1106-
k = glob(i->path, GLOB_NOSORT|GLOB_BRACE|GLOB_ALTDIRFUNC, NULL, &g);
1107-
if (k != 0 && k != GLOB_NOMATCH)
1108-
return log_error_errno(errno ?: EIO, "glob(%s) failed: %m", i->path);
1101+
k = safe_glob(i->path, GLOB_NOSORT|GLOB_BRACE, &g);
1102+
if (k < 0 && k != -ENOENT)
1103+
return log_error_errno(k, "glob(%s) failed: %m", i->path);
11091104

11101105
STRV_FOREACH(fn, g.gl_pathv) {
11111106
k = action(i, *fn);

0 commit comments

Comments
 (0)