Skip to content

Commit efaa1b6

Browse files
millertmillert
committed
handle_restart: Reject log_id if it contains ".."
The log_id is used to create a path name. We need to make sure the ID doesn't start with "../", end with "/..", or contain "/../" to prevent potential path traversal. Thanks to Joshua Rogers for finding this.
1 parent 1d43b6f commit efaa1b6

File tree

5 files changed

+183
-2
lines changed

5 files changed

+183
-2
lines changed

‎MANIFEST‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,7 @@ lib/zlib/zlib.h
419419
lib/zlib/zutil.c
420420
lib/zlib/zutil.h
421421
logsrvd/Makefile.in
422+
logsrvd/dotdot.c
422423
logsrvd/iolog_writer.c
423424
logsrvd/logsrv_util.c
424425
logsrvd/logsrv_util.h
@@ -436,6 +437,7 @@ logsrvd/regress/corpus/seed/logsrvd_conf/logsrvd.conf.4
436437
logsrvd/regress/corpus/seed/logsrvd_conf/logsrvd.conf.5
437438
logsrvd/regress/corpus/seed/logsrvd_conf/logsrvd.conf.6
438439
logsrvd/regress/corpus/seed/logsrvd_conf/logsrvd.conf.7
440+
logsrvd/regress/dotdot/dotdot_test.c
439441
logsrvd/regress/fuzz/fuzz_logsrvd_conf.c
440442
logsrvd/regress/fuzz/fuzz_logsrvd_conf.dict
441443
logsrvd/regress/logsrvd_conf/cacert.pem

‎logsrvd/Makefile.in‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ FUZZ_MAX_LEN = 4096
116116
FUZZ_RUNS = 8192
117117
FUZZ_VERBOSE =
118118

119-
TEST_PROGS = logsrvd_conf_test
119+
TEST_PROGS = logsrvd_conf_test dotdot_test
120120
TEST_LIBS = $(LIBS)
121121
TEST_LDFLAGS = $(LDFLAGS)
122122
TEST_VERBOSE =
@@ -131,7 +131,7 @@ SHELL = @SHELL@
131131

132132
PROGS = sudo_logsrvd sudo_sendlog
133133

134-
LOGSRVD_OBJS = logsrv_util.o iolog_writer.o logsrvd.o logsrvd_conf.o \
134+
LOGSRVD_OBJS = dotdot.o logsrv_util.o iolog_writer.o logsrvd.o logsrvd_conf.o \
135135
logsrvd_journal.o logsrvd_local.o logsrvd_relay.o \
136136
logsrvd_queue.o tls_client.o tls_init.o
137137

@@ -151,6 +151,8 @@ FUZZ_LOGSRVD_CONF_CORPUS = $(srcdir)/regress/corpus/seed/logsrvd_conf/logsrvd.co
151151

152152
CONF_TEST_OBJS = logsrvd_conf_test.o logsrvd_conf.o tls_init.o
153153

154+
DOTDOT_TEST_OBJS = dotdot_test.o dotdot.o
155+
154156
all: $(PROGS)
155157

156158
depend:
@@ -187,6 +189,9 @@ fuzz_logsrvd_conf: $(FUZZ_LOGSRVD_CONF_OBJS) $(LIBFUZZSTUB) $(LT_LIBS)
187189
logsrvd_conf_test: $(CONF_TEST_OBJS) $(LT_LIBS)
188190
$(LIBTOOL) $(LTFLAGS) --mode=link $(CC) -o $@ $(CONF_TEST_OBJS) $(ASAN_LDFLAGS) $(PIE_LDFLAGS) $(HARDENING_LDFLAGS) $(TEST_LDFLAGS) $(TEST_LIBS)
189191

192+
dotdot_test: $(DOTDOT_TEST_OBJS) $(LT_LIBS)
193+
$(LIBTOOL) $(LTFLAGS) --mode=link $(CC) -o $@ $(DOTDOT_TEST_OBJS) $(ASAN_LDFLAGS) $(PIE_LDFLAGS) $(HARDENING_LDFLAGS) $(TEST_LDFLAGS) $(TEST_LIBS)
194+
190195
fuzz_logsrvd_conf_seed_corpus.zip:
191196
tdir=fuzz_logsrvd_conf.$$$$; \
192197
mkdir $$tdir; \
@@ -278,6 +283,7 @@ check: $(TEST_PROGS) check-fuzzer
278283
unset LANGUAGE || LANGUAGE=; \
279284
MALLOC_OPTIONS=S; export MALLOC_OPTIONS; \
280285
MALLOC_CONF="abort:true,junk:true"; export MALLOC_CONF; \
286+
./dotdot_test; \
281287
builddir=$(abs_top_builddir)/logsrvd; \
282288
cd $(srcdir) || exit 1; \
283289
if test -n "@LIBTLS@"; then \
@@ -313,6 +319,30 @@ cleandir: realclean
313319
$(FUZZ_SEED_CORPUS) run-fuzz_logsrvd_conf
314320

315321
# Autogenerated dependencies, do not modify
322+
dotdot.o: $(srcdir)/dotdot.c $(incdir)/compat/stdbool.h \
323+
$(incdir)/sudo_compat.h $(incdir)/sudo_debug.h \
324+
$(incdir)/sudo_queue.h $(srcdir)/logsrv_util.h \
325+
$(top_builddir)/config.h
326+
$(CC) -c $(CPPFLAGS) $(CFLAGS) $(ASAN_CFLAGS) $(PIE_CFLAGS) $(HARDENING_CFLAGS) $(srcdir)/dotdot.c
327+
dotdot.i: $(srcdir)/dotdot.c $(incdir)/compat/stdbool.h \
328+
$(incdir)/sudo_compat.h $(incdir)/sudo_debug.h \
329+
$(incdir)/sudo_queue.h $(srcdir)/logsrv_util.h \
330+
$(top_builddir)/config.h
331+
$(CPP) $(CPPFLAGS) $(srcdir)/dotdot.c > $@
332+
dotdot.plog: dotdot.i
333+
rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/dotdot.c --i-file dotdot.i --output-file $@
334+
dotdot_test.o: $(srcdir)/regress/dotdot/dotdot_test.c \
335+
$(incdir)/compat/stdbool.h $(incdir)/sudo_compat.h \
336+
$(incdir)/sudo_fatal.h $(incdir)/sudo_plugin.h \
337+
$(srcdir)/logsrv_util.h $(top_builddir)/config.h
338+
$(CC) -c $(CPPFLAGS) $(CFLAGS) $(ASAN_CFLAGS) $(PIE_CFLAGS) $(HARDENING_CFLAGS) $(srcdir)/regress/dotdot/dotdot_test.c
339+
dotdot_test.i: $(srcdir)/regress/dotdot/dotdot_test.c \
340+
$(incdir)/compat/stdbool.h $(incdir)/sudo_compat.h \
341+
$(incdir)/sudo_fatal.h $(incdir)/sudo_plugin.h \
342+
$(srcdir)/logsrv_util.h $(top_builddir)/config.h
343+
$(CPP) $(CPPFLAGS) $(srcdir)/regress/dotdot/dotdot_test.c > $@
344+
dotdot_test.plog: dotdot_test.i
345+
rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/regress/dotdot/dotdot_test.c --i-file dotdot_test.i --output-file $@
316346
fuzz_logsrvd_conf.o: $(srcdir)/regress/fuzz/fuzz_logsrvd_conf.c \
317347
$(incdir)/compat/stdbool.h $(incdir)/log_server.pb-c.h \
318348
$(incdir)/protobuf-c/protobuf-c.h $(incdir)/sudo_compat.h \

‎logsrvd/logsrv_util.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ struct connection_buffer {
5353
};
5454
TAILQ_HEAD(connection_buffer_list, connection_buffer);
5555

56+
/* dotdot.c */
57+
bool contains_dot_dot(const char *str);
58+
5659
/* logsrv_util.c */
5760
struct iolog_file;
5861
bool expand_buf(struct connection_buffer *buf, size_t needed);

‎logsrvd/logsrvd.c‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,14 @@ handle_restart(const RestartMessage *msg, const uint8_t *buf, size_t len,
591591
"%s: received RestartMessage for %s from %s", __func__, msg->log_id,
592592
source);
593593

594+
/* The log_id is used to create a path name, prevent path traversal. */
595+
if (contains_dot_dot(msg->log_id)) {
596+
sudo_warnx(U_("%s: %s"), source,
597+
U_("RestartMessage log_id path traversal attack"));
598+
closure->errstr = _("invalid RestartMessage");
599+
debug_return_bool(false);
600+
}
601+
594602
/* Only I/O logs are restartable. */
595603
closure->log_io = true;
596604

‎logsrvd/regress/dotdot/dotdot_test.c‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
/*
2+
* SPDX-License-Identifier: ISC
3+
*
4+
* Copyright (c) 2025 Todd C. Miller <[email protected]>
5+
*
6+
* Permission to use, copy, modify, and distribute this software for any
7+
* purpose with or without fee is hereby granted, provided that the above
8+
* copyright notice and this permission notice appear in all copies.
9+
*
10+
* THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
11+
* WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
12+
* MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
13+
* ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
14+
* WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
15+
* ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
16+
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
17+
*/
18+
19+
#include <config.h>
20+
21+
#include <stdio.h>
22+
#include <stdlib.h>
23+
#include <string.h>
24+
#ifdef HAVE_STDBOOL_H
25+
# include <stdbool.h>
26+
#else
27+
# include <compat/stdbool.h>
28+
#endif /* HAVE_STDBOOL_H */
29+
30+
#define SUDO_ERROR_WRAP 0
31+
32+
#include <sudo_compat.h>
33+
#include <sudo_util.h>
34+
#include <sudo_fatal.h>
35+
#include <sudo_queue.h>
36+
37+
#include <logsrv_util.h>
38+
39+
sudo_dso_public int main(int argc, char *argv[]);
40+
41+
struct test_data {
42+
const char *str; /* input string */
43+
bool expected; /* expected result */
44+
};
45+
46+
static struct test_data test_data[] = {
47+
{
48+
"/foo/bar",
49+
false
50+
}, {
51+
"..",
52+
true
53+
}, {
54+
"../",
55+
true
56+
}, {
57+
"/..",
58+
true
59+
}, {
60+
"/../",
61+
true
62+
}, {
63+
"foo/../",
64+
true
65+
}, {
66+
"foo/..",
67+
true
68+
}, {
69+
"foo/../bar",
70+
true
71+
}, {
72+
"../bar",
73+
true
74+
}, {
75+
"foo../bar",
76+
false
77+
}, {
78+
"foo/..bar",
79+
false
80+
}, {
81+
"...",
82+
false
83+
}, {
84+
".../",
85+
false
86+
}, {
87+
"/...",
88+
false
89+
}, {
90+
"/.../",
91+
false
92+
}, {
93+
NULL,
94+
false
95+
}
96+
};
97+
98+
/*
99+
* Verify contains_dot_dot() behavior
100+
*/
101+
int
102+
main(int argc, char *argv[])
103+
{
104+
int errors = 0, ntests = 0;
105+
size_t i;
106+
int ch;
107+
108+
initprogname(argc > 0 ? argv[0] : "dotdot_test");
109+
110+
while ((ch = getopt(argc, argv, "v")) != -1) {
111+
switch (ch) {
112+
case 'v':
113+
/* ignore */
114+
break;
115+
default:
116+
fprintf(stderr, "usage: %s [-v]\n", getprogname());
117+
return EXIT_FAILURE;
118+
}
119+
}
120+
121+
for (i = 0; test_data[i].str != NULL; i++) {
122+
bool result = contains_dot_dot(test_data[i].str);
123+
if (result != test_data[i].expected) {
124+
sudo_warnx("test %zu:%s: expected %s, got %s", i,
125+
test_data[i].str, test_data[i].expected ? "true" : "false",
126+
result ? "true" : "false");
127+
errors++;
128+
}
129+
ntests++;
130+
}
131+
132+
if (ntests != 0) {
133+
printf("%s: %d tests run, %d errors, %d%% success rate\n",
134+
getprogname(), ntests, errors, (ntests - errors) * 100 / ntests);
135+
}
136+
137+
return errors;
138+
}

0 commit comments

Comments
 (0)