Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Heap corruption (I think freeing uninitialized memory?) with sssd plugin #67

Closed
iagox86 opened this issue Oct 9, 2020 · 2 comments
Closed

Comments

@iagox86
Copy link

iagox86 commented Oct 9, 2020

Hello,

I've been going down a rabbithole trying to debug a crash in sudo. I'm not familiar with the code, but I'll try to give as much info as I can. I believe that sssd is returning an error, and sudo's error handler is handling it badly. I'm doing this on Fedora 32, and it only recently started failing. We rebuilt the system, reinstalled everything, and it still crashes. Here's what our nsswitch.conf file looks:

[root@dev .libs]# cat /etc/nsswitch.conf
passwd:     sss files systemd
group:      sss files systemd
netgroup:   sss files
automount:  sss files
services:   sss files
sudoers:    sss files
[...]

The issue appears to be related to the sssd plugin. When sss is enabled there, sudo immediately crashes with a segfault:

[ron@dev sudo]$ sudo -l
Segmentation fault

If I do that from an older version, but same config, it correctly prints an error:

[root@dev sudo]# sudo -U chris -ll
sudo: error in /etc/sudo.conf, line 16 while loading plugin "sudoers_audit"
sudo: unknown plugin type 3 found in /usr/libexec/sudo/sudoers.so
sudo: fatal error, unable to load plugins

If I disable sss, it also works. I compiled master (607076d) and confirmed it still crashes:

[root@dev sudo]# git checkout master
Already on 'master'
Your branch is up to date with 'origin/master'.
[root@dev sudo]# git rev-parse HEAD
607076d8a0b42688b63d5da52b54715730022586
[root@dev sudo]# make clean && ./configure --prefix=/usr --with-sssd && make -j10

[...compile compile compile...]

[root@dev sudo]# cd ~ron/sudo/src/.libs/; ./sudo -U chris -ll
Segmentation fault

I debugged it:

[root@dev .libs]# gdb --args ./sudo -U chris -ll
(gdb) run
Starting program: /home/ron/sudo/src/.libs/sudo -U chris -ll
Missing separate debuginfos, use: dnf debuginfo-install glibc-2.31-4.fc32.x86_64
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
[Detaching after fork from child process 762346]

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7e34aa0 in free () from /lib64/libc.so.6
Missing separate debuginfos, use: dnf debuginfo-install oddjob-mkhomedir-0.34.6-1.fc32.x86_64
(gdb) bt
#0  0x00007ffff7e34aa0 in free () from /lib64/libc.so.6
#1  0x00007ffff7fc6205 in sss_sudo_free_values (values=<optimized out>) at src/sss_client/sudo/sss_sudo.c:192
#2  sss_sudo_free_values (values=0x7fffffffe660) at src/sss_client/sudo/sss_sudo.c:183
#3  0x00007fffea15baaf in sss_rule_to_priv (rc_out=<synthetic pointer>, rule=0x5555555a2610, handle=0x55555558ff70) at ./sssd.c:336
#4  sss_to_sudoers (sss_result=0x5555555a25e0, handle=0x55555558ff70) at ./sssd.c:398
#5  sudo_sss_query (nss=<optimized out>, pw=<optimized out>) at ./sssd.c:684
#6  0x00007fffea14c860 in display_privs (snl=0x7fffea1b0540 <snl>, pw=0x555555592898, verbose=verbose@entry=true) at ./parse.c:753
#7  0x00007fffea156555 in sudoers_policy_main (argc=argc@entry=0, argv=argv@entry=0x7fffffffec08, pwflag=pwflag@entry=54, env_add=env_add@entry=0x0, verbose=verbose@entry=true, closure=closure@entry=0x0) at ./sudoers.c:560
#8  0x00007fffea14deb2 in sudoers_policy_list (argc=0, argv=0x7fffffffec08, verbose=16777216, list_user=0x7fffffffee3f "chris", errstr=0x7fffffffe998) at ./policy.c:1041
#9  0x000055555555aae1 in policy_list (envp=0x7fffffffec10, user=0x7fffffffee3f "chris", verbose=16777216, argv=0x7fffffffec08, argc=0) at ./sudo.c:1223
#10 main (argc=<optimized out>, argv=<optimized out>, envp=0x7fffffffec10) at ./sudo.c:262

Since it's a potential memory issue, I also checked with valgrind, which sees issues in approximately the same place as gdb (created a gist since it got pretty long):

https://gist.github.com/499832b190b762f6b33770e160c5db3d

Oddly, it doesn't crash with valgrind - maybe because it handles unintialized memory more gracefully?

Here is sudo_debug from running master: https://gist.github.com/0a04421c0cc2df538be2660d8a5882f2

And sudoers_debug from the same execution: https://gist.github.com/f82c4ae2d427d400d8958d6c3d1082d6

And finally, I did a git bisect and found the first crashing commit is 22105009d86a5c252cc3d4d187fb54ac18a6ec0f:

[root@dev .libs]# git checkout 88f9f2ba9a55b3654c829bee69c6dcd7b5feef4d

[compile]

[root@dev sudo]# cd ~ron/sudo/src/.libs/; ./sudo -U chris -ll
sudo: error in /etc/sudo.conf, line 16 while loading plugin "sudoers_audit"
sudo: unknown plugin type 3 found in /usr/libexec/sudo/sudoers.so
sudo: fatal error, unable to load plugins

[root@dev .libs]# git checkout 22105009d86a5c252cc3d4d187fb54ac18a6ec0f

[compile]

# cd ~ron/sudo/src/.libs/; ./sudo -U chris -ll
Segmentation fault

That's all the information I could hope to find. I really hope that's enough, but I'm totally willing to give you whatever additional information (including an account on the box where it's failing or its qcow2 file).

Thanks!

@iagox86
Copy link
Author

iagox86 commented Oct 9, 2020

Sorry, I looked into it a bit more and I think this might be the fix:

# git diff
diff --git a/plugins/sudoers/sssd.c b/plugins/sudoers/sssd.c
index dcc80b96e..6c1f3fc06 100644
--- a/plugins/sudoers/sssd.c
+++ b/plugins/sudoers/sssd.c
@@ -240,7 +240,7 @@ static struct privilege *
 sss_rule_to_priv(struct sudo_sss_handle *handle, struct sss_sudo_rule *rule,
     int *rc_out)
 {
-    char **cmnds, **runasusers = NULL, **runasgroups = NULL;
+    char **cmnds = NULL, **runasusers = NULL, **runasgroups = NULL;
     char **opts = NULL, **notbefore = NULL, **notafter = NULL;
     char **hosts = NULL, **cn_array = NULL, *cn = NULL;
     struct privilege *priv = NULL;

cmnds isn't initialized to NULL in master, but some code assumes it is:

    if (cmnds != NULL)
        handle->fn_free_values(cmnds);

Passing an uninitialized pointer to free is a potential vulnerability, but more importantly it's causing my system to crash. :)

millert added a commit that referenced this issue Oct 9, 2020
In the sssd backend, the rule_to_priv() cleanup code assumes cmnds
can be passed to fn_free_values(), which was not the case if we
receive an error getting values for "sudoCommand".  This is a
regression introduced in sudo 1.9.1.  Fix from Ron Bowes.
GitHub issue #67.
@millert
Copy link
Collaborator

millert commented Oct 9, 2020

Thanks, it looks like this bug was introduced in sudo 1.9.1. I verified that the LDAP backend doesn't have this problem in the equivalent function.

@millert millert closed this as completed Oct 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants