From 2804c2c78e7007c6e718754efb739b7fa8831f0c Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Wed, 27 Jan 2021 15:41:54 -0700 Subject: [PATCH] Add strlcpy_unescape() function to undo escaping from front-end. Includes unit test. --- MANIFEST | 2 + plugins/sudoers/Makefile.in | 56 +++++++++++- .../sudoers/regress/unescape/check_unesc.c | 89 +++++++++++++++++++ plugins/sudoers/strlcpy_unesc.c | 52 +++++++++++ plugins/sudoers/sudoers.c | 51 +++++------ plugins/sudoers/sudoers.h | 3 + 6 files changed, 223 insertions(+), 30 deletions(-) create mode 100644 plugins/sudoers/regress/unescape/check_unesc.c create mode 100644 plugins/sudoers/strlcpy_unesc.c diff --git a/MANIFEST b/MANIFEST index 2d7746da8..8bac5b3bd 100644 --- a/MANIFEST +++ b/MANIFEST @@ -687,6 +687,7 @@ plugins/sudoers/regress/parser/check_fill.c plugins/sudoers/regress/parser/check_gentime.c plugins/sudoers/regress/parser/check_hexchar.c plugins/sudoers/regress/starttime/check_starttime.c +plugins/sudoers/regress/unescape/check_unesc.c plugins/sudoers/regress/sudoers/test1.in plugins/sudoers/regress/sudoers/test1.json.ok plugins/sudoers/regress/sudoers/test1.ldif.ok @@ -884,6 +885,7 @@ plugins/sudoers/solaris_audit.c plugins/sudoers/solaris_audit.h plugins/sudoers/sssd.c plugins/sudoers/starttime.c +plugins/sudoers/strlcpy_unesc.c plugins/sudoers/strlist.c plugins/sudoers/strlist.h plugins/sudoers/stubs.c diff --git a/plugins/sudoers/Makefile.in b/plugins/sudoers/Makefile.in index 5aeeae633..9fe0c831b 100644 --- a/plugins/sudoers/Makefile.in +++ b/plugins/sudoers/Makefile.in @@ -155,7 +155,7 @@ PROGS = sudoers.la visudo sudoreplay cvtsudoers testsudoers TEST_PROGS = check_addr check_base64 check_digest check_env_pattern \ check_exptilde check_fill check_gentime check_hexchar \ - check_iolog_plugin check_starttime @SUDOERS_TEST_PROGS@ + check_iolog_plugin check_starttime check_unesc @SUDOERS_TEST_PROGS@ AUTH_OBJS = sudo_auth.lo @AUTH_OBJS@ @@ -229,6 +229,8 @@ CHECK_SYMBOLS_OBJS = check_symbols.o CHECK_STARTTIME_OBJS = check_starttime.o starttime.lo sudoers_debug.lo +CHECK_UNESC_OBJS = check_unesc.o strlcpy_unesc.lo sudoers_debug.lo + VERSION = @PACKAGE_VERSION@ PACKAGE_TARNAME = @PACKAGE_TARNAME@ @@ -325,6 +327,9 @@ check_iolog_plugin: $(CHECK_IOLOG_PLUGIN_OBJS) $(LIBUTIL) $(LIBIOLOG) $(LIBLOGSR check_starttime: $(CHECK_STARTTIME_OBJS) $(LIBUTIL) $(LIBTOOL) $(LTFLAGS) --mode=link $(CC) -o $@ $(CHECK_STARTTIME_OBJS) $(LDFLAGS) $(ASAN_LDFLAGS) $(PIE_LDFLAGS) $(SSP_LDFLAGS) $(LIBS) +check_unesc: $(CHECK_UNESC_OBJS) $(LIBUTIL) + $(LIBTOOL) $(LTFLAGS) --mode=link $(CC) -o $@ $(CHECK_UNESC_OBJS) $(LDFLAGS) $(ASAN_LDFLAGS) $(PIE_LDFLAGS) $(SSP_LDFLAGS) $(LIBS) + # We need to link check_symbols with -lpthread on HP-UX since LDAP uses threads check_symbols: $(CHECK_SYMBOLS_OBJS) $(LIBUTIL) $(LIBTOOL) $(LTFLAGS) --mode=link $(CC) -o $@ $(CHECK_SYMBOLS_OBJS) $(CHECK_SYMBOLS_LDFLAGS) $(LDFLAGS) $(ASAN_LDFLAGS) $(PIE_LDFLAGS) $(SSP_LDFLAGS) $(LIBS) @SUDO_LIBS@ @@ -473,6 +478,7 @@ check: $(TEST_PROGS) visudo testsudoers cvtsudoers mkdir -p regress/iolog_plugin; \ ./check_iolog_plugin regress/iolog_plugin/iolog || rval=`expr $$rval + $$?`; \ ./check_starttime || rval=`expr $$rval + $$?`; \ + ./check_unesc || rval=`expr $$rval + $$?`; \ if test -f check_symbols; then \ ./check_symbols .libs/sudoers.so $(shlib_exp) || rval=`expr $$rval + $$?`; \ fi; \ @@ -1028,6 +1034,30 @@ check_symbols.i: $(srcdir)/regress/check_symbols/check_symbols.c \ $(CC) -E -o $@ $(CPPFLAGS) $< check_symbols.plog: check_symbols.i rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/regress/check_symbols/check_symbols.c --i-file $< --output-file $@ +check_unesc.o: $(srcdir)/regress/unescape/check_unesc.c $(devdir)/def_data.h \ + $(incdir)/compat/stdbool.h $(incdir)/sudo_compat.h \ + $(incdir)/sudo_conf.h $(incdir)/sudo_debug.h \ + $(incdir)/sudo_eventlog.h $(incdir)/sudo_fatal.h \ + $(incdir)/sudo_gettext.h $(incdir)/sudo_plugin.h \ + $(incdir)/sudo_queue.h $(incdir)/sudo_util.h \ + $(srcdir)/defaults.h $(srcdir)/logging.h $(srcdir)/parse.h \ + $(srcdir)/sudo_nss.h $(srcdir)/sudoers.h \ + $(srcdir)/sudoers_debug.h $(top_builddir)/config.h \ + $(top_builddir)/pathnames.h + $(CC) -c $(CPPFLAGS) $(CFLAGS) $(ASAN_CFLAGS) $(PIE_CFLAGS) $(SSP_CFLAGS) $(srcdir)/regress/unescape/check_unesc.c +check_unesc.i: $(srcdir)/regress/unescape/check_unesc.c $(devdir)/def_data.h \ + $(incdir)/compat/stdbool.h $(incdir)/sudo_compat.h \ + $(incdir)/sudo_conf.h $(incdir)/sudo_debug.h \ + $(incdir)/sudo_eventlog.h $(incdir)/sudo_fatal.h \ + $(incdir)/sudo_gettext.h $(incdir)/sudo_plugin.h \ + $(incdir)/sudo_queue.h $(incdir)/sudo_util.h \ + $(srcdir)/defaults.h $(srcdir)/logging.h $(srcdir)/parse.h \ + $(srcdir)/sudo_nss.h $(srcdir)/sudoers.h \ + $(srcdir)/sudoers_debug.h $(top_builddir)/config.h \ + $(top_builddir)/pathnames.h + $(CC) -E -o $@ $(CPPFLAGS) $< +check_unesc.plog: check_unesc.i + rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/regress/unescape/check_unesc.c --i-file $< --output-file $@ cvtsudoers.o: $(srcdir)/cvtsudoers.c $(devdir)/def_data.h $(devdir)/gram.h \ $(incdir)/compat/getopt.h $(incdir)/compat/stdbool.h \ $(incdir)/sudo_compat.h $(incdir)/sudo_conf.h \ @@ -2352,6 +2382,30 @@ starttime.i: $(srcdir)/starttime.c $(devdir)/def_data.h \ $(CC) -E -o $@ $(CPPFLAGS) $< starttime.plog: starttime.i rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/starttime.c --i-file $< --output-file $@ +strlcpy_unesc.lo: $(srcdir)/strlcpy_unesc.c $(devdir)/def_data.h \ + $(incdir)/compat/stdbool.h $(incdir)/sudo_compat.h \ + $(incdir)/sudo_conf.h $(incdir)/sudo_debug.h \ + $(incdir)/sudo_eventlog.h $(incdir)/sudo_fatal.h \ + $(incdir)/sudo_gettext.h $(incdir)/sudo_plugin.h \ + $(incdir)/sudo_queue.h $(incdir)/sudo_util.h \ + $(srcdir)/defaults.h $(srcdir)/logging.h $(srcdir)/parse.h \ + $(srcdir)/sudo_nss.h $(srcdir)/sudoers.h \ + $(srcdir)/sudoers_debug.h $(top_builddir)/config.h \ + $(top_builddir)/pathnames.h + $(LIBTOOL) $(LTFLAGS) --mode=compile $(CC) -c $(CPPFLAGS) $(CFLAGS) $(ASAN_CFLAGS) $(PIE_CFLAGS) $(SSP_CFLAGS) $(srcdir)/strlcpy_unesc.c +strlcpy_unesc.i: $(srcdir)/strlcpy_unesc.c $(devdir)/def_data.h \ + $(incdir)/compat/stdbool.h $(incdir)/sudo_compat.h \ + $(incdir)/sudo_conf.h $(incdir)/sudo_debug.h \ + $(incdir)/sudo_eventlog.h $(incdir)/sudo_fatal.h \ + $(incdir)/sudo_gettext.h $(incdir)/sudo_plugin.h \ + $(incdir)/sudo_queue.h $(incdir)/sudo_util.h \ + $(srcdir)/defaults.h $(srcdir)/logging.h $(srcdir)/parse.h \ + $(srcdir)/sudo_nss.h $(srcdir)/sudoers.h \ + $(srcdir)/sudoers_debug.h $(top_builddir)/config.h \ + $(top_builddir)/pathnames.h + $(CC) -E -o $@ $(CPPFLAGS) $< +strlcpy_unesc.plog: strlcpy_unesc.i + rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/strlcpy_unesc.c --i-file $< --output-file $@ strlist.lo: $(srcdir)/strlist.c $(incdir)/compat/stdbool.h \ $(incdir)/sudo_compat.h $(incdir)/sudo_debug.h \ $(incdir)/sudo_queue.h $(incdir)/sudo_util.h $(srcdir)/strlist.h \ diff --git a/plugins/sudoers/regress/unescape/check_unesc.c b/plugins/sudoers/regress/unescape/check_unesc.c new file mode 100644 index 000000000..b955eaaee --- /dev/null +++ b/plugins/sudoers/regress/unescape/check_unesc.c @@ -0,0 +1,89 @@ +/* + * SPDX-License-Identifier: ISC + * + * Copyright (c) 2021 Todd C. Miller + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#include + +#include +#include +#include + +#define SUDO_ERROR_WRAP 0 + +#include "sudoers.h" + +struct test_data { + char *input; + char *result; + size_t result_len; + size_t bufsize; +} test_data[] = { + { "\\\0ABC", "\\", 1, 2 }, /* 1 */ + { "\\ \\;", "\\ ;", 3, 4 }, /* 2 */ + { "\\\t\\;", "\\\t;", 3, 4 }, /* 3 */ + { "\\foo", "foo", 3, 4 }, /* 4 */ + { "foo\\ bar", "foo\\ bar", 8, 9 }, /* 5 */ + { "foo bar", "f", 7, 2 }, /* 6 */ + { "foo bar", "", 7, 1 }, /* 7 */ + { "foo bar", NULL, 7, 0 }, /* 8 */ + { NULL } +}; + +sudo_dso_public int main(int argc, char *argv[]); + +int +main(int argc, char *argv[]) +{ + int ntests = 0, errors = 0; + struct test_data *td; + char buf[1024]; + size_t len; + + initprogname(argc > 0 ? argv[0] : "check_unesc"); + + for (td = test_data; td->input != NULL; td++) { + ntests++; + memset(buf, 'A', sizeof(buf)); + len = strlcpy_unescape(buf, td->input, td->bufsize); + if (len != td->result_len) { + sudo_warnx("%d: \"%s\": bad return %zu, expected %zu", + ntests, td->input, len, td->result_len); + errors++; + } + len = td->result ? strlen(td->result) : 0; + if ((len != 0 || td->bufsize != 0) && len >= td->bufsize) { + sudo_warnx("%d: \"%s\": bad length %zu >= %zu", + ntests, td->input, len, td->bufsize); + errors++; + } + if (td->result != NULL && strcmp(td->result, buf) != 0) { + sudo_warnx("%d: \"%s\": got \"%s\", expected \"%s\"", + ntests, td->input, buf, td->result); + errors++; + } + if (buf[td->bufsize] != 'A') { + sudo_warnx("%d: \"%s\": wrote past end of buffer at %zu (0x%x)", + ntests, td->input, td->bufsize, buf[td->bufsize]); + errors++; + } + } + + printf("%s: %d tests run, %d errors, %d%% success rate\n", getprogname(), + ntests, errors, (ntests - errors) * 100 / ntests); + + exit(errors); +} diff --git a/plugins/sudoers/strlcpy_unesc.c b/plugins/sudoers/strlcpy_unesc.c new file mode 100644 index 000000000..ee6276e76 --- /dev/null +++ b/plugins/sudoers/strlcpy_unesc.c @@ -0,0 +1,52 @@ +/* + * SPDX-License-Identifier: ISC + * + * Copyright (c) 2021 Todd C. Miller + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +/* + * This is an open source non-commercial project. Dear PVS-Studio, please check it. + * PVS-Studio Static Code Analyzer for C, C++ and C#: http://www.viva64.com + */ + +#include + +#include +#include +#include + +#include "sudoers.h" + +size_t +strlcpy_unescape(char *dst, const char *src, size_t size) +{ + size_t len = 0; + char ch; + debug_decl(strlcpy_unescape, SUDOERS_DEBUG_UTIL); + + while ((ch = *src++) != '\0') { + if (ch == '\\' && *src != '\0' && !isspace((unsigned char)*src)) + ch = *src++; + if (size > 1) { + *dst++ = ch; + size--; + } + len++; + } + if (size > 0) + *dst = '\0'; + + debug_return_size_t(len); +} diff --git a/plugins/sudoers/sudoers.c b/plugins/sudoers/sudoers.c index 20f760bc6..35c243e53 100644 --- a/plugins/sudoers/sudoers.c +++ b/plugins/sudoers/sudoers.c @@ -951,7 +951,7 @@ set_cmnd(void) /* set user_args */ if (NewArgc > 1) { - char *to, *from, **av; + char *dst, **av; size_t size, n; /* Alloc and build up user_args. */ @@ -968,38 +968,31 @@ set_cmnd(void) * escapes potential meta chars. We unescape non-spaces * for sudoers matching and logging purposes. */ - for (to = user_args, av = NewArgv + 1; (from = *av); av++) { - while (*from) { - if (from[0] == '\\' && from[1] != '\0' && - !isspace((unsigned char)from[1])) { - from++; - } - if (size - (to - user_args) < 1) { - sudo_warnx(U_("internal error, %s overflow"), - __func__); - debug_return_int(NOT_FOUND_ERROR); - } - *to++ = *from++; - } - if (size - (to - user_args) < 1) { - sudo_warnx(U_("internal error, %s overflow"), - __func__); - debug_return_int(NOT_FOUND_ERROR); - } - *to++ = ' '; - } - *--to = '\0'; - } else { - for (to = user_args, av = NewArgv + 1; *av; av++) { - n = strlcpy(to, *av, size - (to - user_args)); - if (n >= size - (to - user_args)) { + for (dst = user_args, av = NewArgv + 1; *av; av++) { + n = strlcpy_unescape(dst, *av, size); + if (n >= size) { sudo_warnx(U_("internal error, %s overflow"), __func__); debug_return_int(NOT_FOUND_ERROR); } - to += n; - *to++ = ' '; + dst += n; + size -= n; + *dst++ = ' '; + size--; } - *--to = '\0'; + *--dst = '\0'; + } else { + for (dst = user_args, av = NewArgv + 1; *av; av++) { + n = strlcpy(dst, *av, size); + if (n >= size) { + sudo_warnx(U_("internal error, %s overflow"), __func__); + debug_return_int(NOT_FOUND_ERROR); + } + dst += n; + size -= n; + *dst++ = ' '; + size--; + } + *--dst = '\0'; } } } diff --git a/plugins/sudoers/sudoers.h b/plugins/sudoers/sudoers.h index bece1ebad..160540d9e 100644 --- a/plugins/sudoers/sudoers.h +++ b/plugins/sudoers/sudoers.h @@ -455,4 +455,7 @@ char *rcstr_alloc(size_t len); char *rcstr_addref(const char *s); void rcstr_delref(const char *s); +/* strlcpy_unesc.c */ +size_t strlcpy_unescape(char *dst, const char *src, size_t size); + #endif /* SUDOERS_SUDOERS_H */