From b1c450877fba9f0f6166b9dfd6483082d8fe3d02 Mon Sep 17 00:00:00 2001 From: iefa m Date: Sat, 24 Oct 2026 15:09:00 +0530 Subject: [PATCH] sudo_ldap_role_to_priv: treat an invalid sudoNotAfter as expired parse_gentime() returns -1 on error, which is also the value of UNSPEC, so a sudoNotBefore or sudoNotAfter value that cannot be parsed made the sudoRole unrestricted instead of time-limited. parse_gentime() only accepted one fraction digit, so a valid GeneralizedTime such as 20200101000000.00Z took this path and an expired role kept working. The sudoers grammar already rejects an invalid NOTBEFORE/NOTAFTER value. Treat a role with an unparseable time as expired, warning when warnings are enabled, and let parse_gentime() skip any number of fraction digits. Add check_gentime cases and a testsudoers regression test. --- MANIFEST | 2 + plugins/sudoers/gentime.c | 4 +- plugins/sudoers/ldap_util.c | 32 ++++++++++++++-- .../sudoers/regress/parser/check_gentime.c | 4 ++ .../sudoers/regress/testsudoers/test34.out.ok | 38 +++++++++++++++++++ plugins/sudoers/regress/testsudoers/test34.sh | 27 +++++++++++++ 6 files changed, 103 insertions(+), 4 deletions(-) create mode 100644 plugins/sudoers/regress/testsudoers/test34.out.ok create mode 100755 plugins/sudoers/regress/testsudoers/test34.sh diff --git a/MANIFEST b/MANIFEST index 9f54e05edd..8ef4c345ea 100644 --- a/MANIFEST +++ b/MANIFEST @@ -1135,6 +1135,8 @@ plugins/sudoers/regress/testsudoers/test32.out.ok plugins/sudoers/regress/testsudoers/test32.sh plugins/sudoers/regress/testsudoers/test33.out.ok plugins/sudoers/regress/testsudoers/test33.sh +plugins/sudoers/regress/testsudoers/test34.out.ok +plugins/sudoers/regress/testsudoers/test34.sh plugins/sudoers/regress/testsudoers/test4.out.ok plugins/sudoers/regress/testsudoers/test4.sh plugins/sudoers/regress/testsudoers/test5.out.ok diff --git a/plugins/sudoers/gentime.c b/plugins/sudoers/gentime.c index fc35827dd4..2a4526f025 100644 --- a/plugins/sudoers/gentime.c +++ b/plugins/sudoers/gentime.c @@ -93,7 +93,9 @@ parse_gentime(const char *timestr) /* ignore fractional second */ break; } - cp += 2; /* skip over radix and fraction */ + /* skip over radix and fraction, only the first digit is used */ + for (cp += 2; isdigit((unsigned char)*cp); cp++) + continue; } /* Parse optional time zone. */ diff --git a/plugins/sudoers/ldap_util.c b/plugins/sudoers/ldap_util.c index 4ce0ea179b..0310dc79b0 100644 --- a/plugins/sudoers/ldap_util.c +++ b/plugins/sudoers/ldap_util.c @@ -355,6 +355,7 @@ sudo_ldap_role_to_priv(const char *cn, void *hosts, void *runasusers, struct cmndspec *prev_cmndspec = NULL; struct privilege *priv; struct member *m; + bool bad_time = false; char *cmnd; debug_decl(sudo_ldap_role_to_priv, SUDOERS_DEBUG_LDAP); @@ -457,11 +458,36 @@ sudo_ldap_role_to_priv(const char *cn, void *hosts, void *runasusers, goto oom; } - /* Parse sudoNotBefore / sudoNotAfter */ - if (notbefore != NULL) + /* + * Parse sudoNotBefore / sudoNotAfter. + * The parse_gentime() error value is the same as UNSPEC, + * so an invalid value would otherwise remove the restriction. + * Treat an entry with an invalid time as having expired. + */ + if (notbefore != NULL) { cmndspec->notbefore = parse_gentime(notbefore); - if (notafter != NULL) + if (cmndspec->notbefore == -1) { + if (warnings) { + sudo_warnx(U_("invalid sudoNotBefore value: %s"), + notbefore); + } + bad_time = true; + } + } + if (notafter != NULL) { cmndspec->notafter = parse_gentime(notafter); + if (cmndspec->notafter == -1) { + if (warnings) { + sudo_warnx(U_("invalid sudoNotAfter value: %s"), + notafter); + } + bad_time = true; + } + } + if (bad_time) { + cmndspec->notbefore = UNSPEC; + cmndspec->notafter = 0; + } /* Parse sudoOptions. */ if (opts != NULL) { diff --git a/plugins/sudoers/regress/parser/check_gentime.c b/plugins/sudoers/regress/parser/check_gentime.c index 8b1e0f6505..2854dc7cb1 100644 --- a/plugins/sudoers/regress/parser/check_gentime.c +++ b/plugins/sudoers/regress/parser/check_gentime.c @@ -52,6 +52,10 @@ const struct gentime_test { { "201702142359,4Z", 1487116764 }, { "2017021408.5Z", 1487061000 }, { "2017021408,5Z", 1487061000 }, + { "20170214083000.123Z", 1487061000 }, + { "201702142359.45Z", 1487116764 }, + { "2017021408.50-0500", 1487079000 }, + { "20170214083000.Z", -1 }, { "20170214Z", -1 }, }; diff --git a/plugins/sudoers/regress/testsudoers/test34.out.ok b/plugins/sudoers/regress/testsudoers/test34.out.ok new file mode 100644 index 0000000000..8dccf2a9e0 --- /dev/null +++ b/plugins/sudoers/regress/testsudoers/test34.out.ok @@ -0,0 +1,38 @@ +Parses OK + +Entries for user root: + +ALL = (ALL) NOTAFTER=20200101000000Z /bin/sh + host allowed + date denied + +Password required + +Command unmatched + +testsudoers: invalid sudoNotAfter value: bogus +Parses OK + +Entries for user root: + +ALL = (ALL) NOTAFTER=19700101000000Z /bin/sh + host allowed + date denied + +Password required + +Command unmatched + +testsudoers: invalid sudoNotBefore value: bogus +Parses OK + +Entries for user root: + +ALL = (ALL) NOTAFTER=19700101000000Z /bin/sh + host allowed + date denied + +Password required + +Command unmatched + diff --git a/plugins/sudoers/regress/testsudoers/test34.sh b/plugins/sudoers/regress/testsudoers/test34.sh new file mode 100755 index 0000000000..026f73c7ec --- /dev/null +++ b/plugins/sudoers/regress/testsudoers/test34.sh @@ -0,0 +1,27 @@ +#!/bin/sh +# +# Verify that an LDAP sudoNotBefore or sudoNotAfter value that cannot +# be parsed does not remove the time restriction. +# + +: ${TESTSUDOERS=testsudoers} + +exec 2>&1 + +for attr in "sudoNotAfter: 20200101000000.00Z" "sudoNotAfter: bogus" \ + "sudoNotBefore: bogus"; do + $TESTSUDOERS -i ldif root /bin/sh <<-EOF + dn: cn=root,ou=SUDOers,dc=sudo,dc=ws + objectClass: top + objectClass: sudoRole + cn: root + sudoUser: root + sudoRunAs: ALL + sudoHost: ALL + sudoCommand: /bin/sh + $attr + EOF + echo "" +done + +exit 0