Bug 553

Summary: pam_mount.c:417 modify_pm_count: Assertion `user != ((void *)0)' failed
Product: Sudo Reporter: TJ <sudo>
Component: SudoersAssignee: Todd C. Miller <Todd.Miller>
Status: RESOLVED FIXED    
Severity: normal    
Priority: low    
Version: 1.8.3   
Hardware: PC   
OS: Linux   
Attachments: Patch to call pam_open_session() before fork()

Description TJ 2012-04-20 08:45:52 MDT
I've been analysing a bug affecting Ubuntu Precise which uses v1.8.3p1 but, from looking at the hg repository, affects the current HEAD too.

PAM is configured with

session optional pam_mount.so

When sudoer has a valid timestamp it doesn't call pam_open_session() because plugins/sudoers/auth/pam.c::pam_verify() isn't executed.

However at the end of the session sudoer still calls pam_close_session() which tries to call pam_mount's modify_pm_count() with a NULL pointer which triggers the assert and abort:

pam_mount.c:417 modify_pm_count: Assertion `user != ((void *)0)' failed

I think this is an upshot of the re-factored modular plug-in architecture but I'm not familiar enough with sudo and its security concerns to suggest a patch.

I've documented my analysis in detail on the Ubuntu bug 927828 with gdb traces and source-code path analysis starting from comment #11. I'd rather not duplicate it here because further findings would leave one or the other report out of sync.

https://bugs.launchpad.net/ubuntu/+source/libpam-mount/+bug/927828/comments/11
Comment 1 Todd C. Miller 2012-04-20 09:11:15 MDT
I don't think your analysis is correct.  Regardless of whether or not verify_user() is called, pam_start() is called via sudo_auth_init() in check_user() and pam_open_session() is called via sudo_auth_begin_session().

I will download the Ubuntu 12.04 beta and try to reproduce the problem but I would appreciate it if you could verify that the problem still exists with the sudo 1.8.5 release candidate.  See http://www.sudo.ws/sudo/devel.html for links to the source tarball and binary packages.
Comment 2 TJ 2012-04-20 12:46:28 MDT
Thanks, I'll double-check here and test the RC.

I installed breakpoints in gdb for:

Function "pam_begin_session" not defined.
Breakpoint 1 (pam_begin_session) pending.
Function "pam_end_session" not defined.
Breakpoint 2 (pam_end_session) pending.

And only pam_end_session was triggered:

Breakpoint 2, pam_end_session (pw=0x20fa938, auth=0x7fc7cbf4a620) at /build/buildd/sudo-1.8.3p1/plugins/sudoers/auth/pam.c:243
Comment 3 TJ 2012-04-20 18:03:11 MDT
$ sudo -V
Sudo version 1.8.5
Sudoers policy plugin version 1.8.5
Sudoers file grammar version 41
Sudoers I/O plugin version 1.8.5
$ sudo echo TEST
TEST
sudo: pam_mount.c:417: modify_pm_count: Assertion `user != ((void *)0)' failed.
Aborted

Note: As this only seems to affect pam_mount (possibly it is the only pam module to use an assert) I also posted a bug to its tracker but the author investigated and thinks the issue is with sudo.

https://sourceforge.net/tracker/?func=detail&aid=3519691&group_id=41452&atid=430593

You guys know the code better than I, but I'll keep on digging for clues.
Comment 4 Todd C. Miller 2012-04-20 21:38:20 MDT
sudo calls pam_open_session() after it calls fork() (but before the uid is changed), which appears to be what makes pam_mount unhappy.  This is intentional as we don't want pam to modify resource limits, etc of the parent process.  I will do some testing of a change to call it before the fork() and get back to you.
Comment 5 Todd C. Miller 2012-04-20 21:53:54 MDT
Created attachment 341 [details]
Patch to call pam_open_session() before fork()

Please give this a try and see if it solves the problem for you.  I'm not able to reproduce the pam_mount problem in a repeatable manner.
Comment 6 TJ 2012-04-21 04:27:00 MDT
I applied the patch on top of the current tip and it works successfully.

I add in the debian/ packaging directory and build using the standard Debian/Ubuntu build mechanism:

fakeroot debian/rules binary

I have pkg-create-dbgsym installed so each build generates a separate package containing the debug symbols.

The test now works repeatedly without an Abort:

sudo echo TEST
sudo echo TEST

Running it under control of gdb we see confirmation now:

$ sudo LD_PRELOAD=/lib/x86_64-linux-gnu/libpthread.so.0 gdb -x /home/tj/gdb-sudo.cmd --args sudo echo TEST
[sudo] password for tj: 
GNU gdb (Ubuntu/Linaro 7.4-2012.02-0ubuntu2) 7.4-2012.02
Copyright (C) 2012 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
For bug reporting instructions, please see:
<http://bugs.launchpad.net/gdb-linaro/>...
Reading symbols from /usr/bin/sudo...Reading symbols from /usr/lib/debug/usr/bin/sudoedit...done.
done.
Catchpoint 1 (exec)
Catchpoint 2 (fork)
Breakpoint 3 at 0x40c990: file /home/all/SourceCode/sudo/sudo/src/sudo.c, line 1041.
Breakpoint 4 at 0x406710: file /home/all/SourceCode/sudo/sudo/src/exec_common.c, line 138.
Function "fork_cmnd" not defined.
Breakpoint 5 (fork_cmnd) pending.
Breakpoint 6 at 0x407ee0: file /home/all/SourceCode/sudo/sudo/src/exec_pty.c, line 510.
Breakpoint 7 at 0x40c4c0: file /home/all/SourceCode/sudo/sudo/src/sudo.c, line 877.
Breakpoint 8 at 0x40cbc0: file /home/all/SourceCode/sudo/sudo/src/sudo.c, line 1175.
Function "sudoers_policy_main" not defined.
Breakpoint 9 (sudoers_policy_main) pending.
Function "sudoers_policy_init_session" not defined.
Breakpoint 10 (sudoers_policy_init_session) pending.
Function "sudo_auth_begin_session" not defined.
Breakpoint 11 (sudo_auth_begin_session) pending.
Function "pam_open_session" not defined.
Breakpoint 12 (pam_open_session) pending.
Function "pam_close_session" not defined.
Breakpoint 13 (pam_close_session) pending.
Function "pam_begin_session" not defined.
Breakpoint 14 (pam_begin_session) pending.
Function "pam_end_session" not defined.
Breakpoint 15 (pam_end_session) pending.
Function "pam_sm_close_session" not defined.
Breakpoint 16 (pam_sm_close_session) pending.
Function "modify_pm_count" not defined.
Breakpoint 17 (modify_pm_count) pending.
(gdb) run
Starting program: /usr/bin/sudo echo TEST
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".

Breakpoint 9, sudoers_policy_main (argc=2, argv=0x7fff7e9df2c0, pwflag=0, env_add=0x9f48e0, command_infop=0x7fff7e9df178, 
    argv_out=0x7fff7e9df180, user_env_out=0x7fff7e9df188) at /home/all/SourceCode/sudo/sudo/plugins/sudoers/sudoers.c:305
305	{
(gdb) c
Continuing.

Breakpoint 3, run_command (details=0x7fff7e9df0d0) at /home/all/SourceCode/sudo/sudo/src/sudo.c:1041
1041	{
(gdb) c
Continuing.

Breakpoint 8, policy_init_session (details=0x7fff7e9df0d0) at /home/all/SourceCode/sudo/sudo/src/sudo.c:1175
1175	{
(gdb) c
Continuing.

Breakpoint 10, sudoers_policy_init_session (pwd=0xa16a30, user_env=0x7fff7e9df140)
    at /home/all/SourceCode/sudo/sudo/plugins/sudoers/sudoers.c:287
287	{
(gdb) c
Continuing.

Breakpoint 11, sudo_auth_begin_session (pw=0xa16a30, user_env=0x7fff7e9df140)
    at /home/all/SourceCode/sudo/sudo/plugins/sudoers/auth/sudo_auth.c:290
290	{
(gdb) c
Continuing.

Breakpoint 12, pam_open_session (pamh=0x9f8660, flags=0) at pam_session.c:12
12	pam_session.c: No such file or directory.
(gdb) c
Continuing.

Breakpoint 17, modify_pm_count (user=0xa17e20 "root", operation=0x7fa8ea73a751 "1", config=<optimized out>) at pam_mount.c:408
408	static int modify_pm_count(struct config *config, char *user,
(gdb) c
Continuing.

Catchpoint 2 (forked process 10151), 0x00007fa8ec3fb126 in fork () from /lib/x86_64-linux-gnu/libc.so.6


I'll investigate if the patch can be easily/safely applied to Ubuntu's 1.8.3p1 since Precise packages are now frozen leading up to the release end of this month.

Many thanks for the swift fix.
Comment 7 Todd C. Miller 2012-05-17 11:10:25 MDT
Fixed in sudo 1.8.5.