Skip to content

Commit

Permalink
runas_userlist_matches: fix matching a Runas_Spec with an empty runas…
Browse files Browse the repository at this point in the history
… user.

We should only match a rule with an empty runas user if a group was
specified on the command line (sudo -g) without a user (no -u option)
or the user specified their own name on the command line.
GitHub issue #290
  • Loading branch information
millert committed Jul 15, 2023
1 parent 78b7121 commit db704c2
Show file tree
Hide file tree
Showing 4 changed files with 226 additions and 2 deletions.
2 changes: 2 additions & 0 deletions MANIFEST
Original file line number Diff line number Diff line change
Expand Up @@ -1098,6 +1098,8 @@ plugins/sudoers/regress/testsudoers/test26.out.ok
plugins/sudoers/regress/testsudoers/test26.sh
plugins/sudoers/regress/testsudoers/test27.out.ok
plugins/sudoers/regress/testsudoers/test27.sh
plugins/sudoers/regress/testsudoers/test28.out.ok
plugins/sudoers/regress/testsudoers/test28.sh
plugins/sudoers/regress/testsudoers/test3.out.ok
plugins/sudoers/regress/testsudoers/test3.sh
plugins/sudoers/regress/testsudoers/test4.out.ok
Expand Down
10 changes: 8 additions & 2 deletions plugins/sudoers/match.c
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,14 @@ runas_userlist_matches(const struct sudoers_parse_tree *parse_tree,
user_matched = !m->negated;
break;
case MYSELF:
if (!ISSET(sudo_user.flags, RUNAS_USER_SPECIFIED) ||
strcmp(user_name, runas_pw->pw_name) == 0)
/*
* Only match a rule with an empty runas user if a group
* was specified on the command line without a user _or_
* the user specified their own name on the command line.
*/
if ((!ISSET(sudo_user.flags, RUNAS_USER_SPECIFIED) &&
ISSET(sudo_user.flags, RUNAS_GROUP_SPECIFIED)) ||
strcmp(user_name, runas_pw->pw_name) == 0)
user_matched = !m->negated;
break;
}
Expand Down
117 changes: 117 additions & 0 deletions plugins/sudoers/regress/testsudoers/test28.out.ok
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
This should match the 'ALL=ALL' rule.
Parses OK

Entries for user admin:

ALL = (admin : staff) NOPASSWD: ALL
host allowed
runas unmatched

ALL = ALL
host allowed
runas allowed
cmnd allowed

Command allowed

This should match the 'ALL=ALL' rule.
Parses OK

Entries for user admin:

ALL = ALL
host allowed
runas allowed
cmnd allowed

Command allowed

This should match the 'ALL=(:staff) NOPASSWD: ALL' rule.
Parses OK

Entries for user admin:

ALL = (admin : staff) NOPASSWD: ALL
host allowed
runas allowed
cmnd allowed

Command allowed

This should match the 'ALL=(:staff) NOPASSWD: ALL' rule.
Parses OK

Entries for user admin:

ALL = ALL
host allowed
runas unmatched

ALL = (admin : staff) NOPASSWD: ALL
host allowed
runas allowed
cmnd allowed

Command allowed

This should match the 'ALL=(:staff) NOPASSWD: ALL' rule.
Parses OK

Entries for user admin:

ALL = ALL
host allowed
runas unmatched

ALL = (admin : staff) NOPASSWD: ALL
host allowed
runas allowed
cmnd allowed

Command allowed

This should match the 'ALL=(:staff) NOPASSWD: ALL' rule.
Parses OK

Entries for user admin:

ALL = ALL
host allowed
runas unmatched

ALL = (admin : staff) NOPASSWD: ALL
host allowed
runas allowed
cmnd allowed

Command allowed

This should not match any rules.
Parses OK

Entries for user admin:

ALL = ALL
host allowed
runas unmatched

ALL = (admin : staff) NOPASSWD: ALL
host allowed
runas unmatched

Command unmatched

This should not match any rules.
Parses OK

Entries for user admin:

ALL = ALL
host allowed
runas unmatched

ALL = (admin : users) NOPASSWD: ALL
host allowed
runas unmatched

Command unmatched
99 changes: 99 additions & 0 deletions plugins/sudoers/regress/testsudoers/test28.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
#!/bin/sh
#
# Verify that a rule with an empty Runas user matches correctly.
#

: ${TESTSUDOERS=testsudoers}

exec 2>&1

status=0

echo "This should match the 'ALL=ALL' rule."
$TESTSUDOERS -p ${TESTDIR}/passwd -P ${TESTDIR}/group \
admin /bin/ls <<'EOF'
admin ALL = ALL
ALL ALL=(:staff) NOPASSWD: ALL
EOF
if [ $? -ne 0 ]; then
status=1
fi

echo ""
echo "This should match the 'ALL=ALL' rule."
$TESTSUDOERS -p ${TESTDIR}/passwd -P ${TESTDIR}/group \
admin /bin/ls <<'EOF'
ALL ALL=(:staff) NOPASSWD: ALL
admin ALL = ALL
EOF
if [ $? -ne 0 ]; then
status=1
fi

echo ""
echo "This should match the 'ALL=(:staff) NOPASSWD: ALL' rule."
$TESTSUDOERS -p ${TESTDIR}/passwd -P ${TESTDIR}/group -g staff \
admin /bin/ls <<'EOF'
admin ALL = ALL
ALL ALL=(:staff) NOPASSWD: ALL
EOF
if [ $? -ne 0 ]; then
status=1
fi

echo ""
echo "This should match the 'ALL=(:staff) NOPASSWD: ALL' rule."
$TESTSUDOERS -p ${TESTDIR}/passwd -P ${TESTDIR}/group -g staff \
admin /bin/ls <<'EOF'
ALL ALL=(:staff) NOPASSWD: ALL
admin ALL = ALL
EOF
if [ $? -ne 0 ]; then
status=1
fi

echo ""
echo "This should match the 'ALL=(:staff) NOPASSWD: ALL' rule."
$TESTSUDOERS -p ${TESTDIR}/passwd -P ${TESTDIR}/group -u admin \
admin /bin/ls <<'EOF'
ALL ALL=(:staff) NOPASSWD: ALL
admin ALL = ALL
EOF
if [ $? -ne 0 ]; then
status=1
fi

echo ""
echo "This should match the 'ALL=(:staff) NOPASSWD: ALL' rule."
$TESTSUDOERS -p ${TESTDIR}/passwd -P ${TESTDIR}/group -u admin -g staff \
admin /bin/ls <<'EOF'
ALL ALL=(:staff) NOPASSWD: ALL
admin ALL = ALL
EOF
if [ $? -ne 0 ]; then
status=1
fi

echo ""
echo "This should not match any rules."
$TESTSUDOERS -p ${TESTDIR}/passwd -P ${TESTDIR}/group -g guest \
admin /bin/ls <<'EOF'
ALL ALL=(:staff) NOPASSWD: ALL
admin ALL = ALL
EOF
if [ $? -eq 0 ]; then
status=1
fi

echo ""
echo "This should not match any rules."
$TESTSUDOERS -p ${TESTDIR}/passwd -P ${TESTDIR}/group -u root -g users \
admin /bin/ls <<'EOF'
ALL ALL=(:users) NOPASSWD: ALL
admin ALL = ALL
EOF
if [ $? -eq 0 ]; then
status=1
fi

exit $status

0 comments on commit db704c2

Please sign in to comment.