adsys pam issues
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
adsys (Ubuntu) |
Fix Released
|
Undecided
|
Unassigned | ||
Focal |
Fix Released
|
Undecided
|
Unassigned |
Bug Description
[Impact]
Memory leaks in adsys pam modules.
[Test Plan]
1. Install SRU version of adsys
2. Login as an user
3. Ensure that you can still login successfully.
[Where problems could occur]
Login can be disabled due to the PAM module crashing. There is only one code path leading to that, so easy to detect.
--------------
These may not be security issues but it's possible I overlooked something; since they live in a security boundary I thought it worth reporting with a bit of hassle. If you'd rather work on this in the open, feel free to open this.
pam_adsys.c update_policy() arggv leak in fork() failure
pam_adsys.c update_
pam_adsys.c update_
work but I don't think that's how that API is supposed to be used
pam_adsys.c pam_sm_
Thanks
Changed in adsys (Ubuntu): | |
status: | New → Fix Committed |
description: | updated |
Yeah, I don’t really think those are security issues, let’s open it, but fix some of them:
-> the 2 arggv leak in fork()
This is only if a failure and just before exiting the child process. Anyway, let’s free it :)
-> status != 0
w = waitpid(cpid, &wstatus, WUNTRACED | WCONTINUED);
if (w == -1) {
perror( "waitpid" );
exit( EXIT_FAILURE) ;
}
Yes, we could have done something like the man page:
do {
} while (!WIFEXITED(
However, this seems equivalent to me. We took our inspiration from pam_exec() with the different kinds of errors and this is the way they are using it, so we did something similar as we are running in the same context. It ought to work as we tested different kind of failures manually and used scenarios worked. Note that pam_exec don’t even free argvv for the parent process :p
So, depending on how strongly you feel about it, tell us!
-> gethostname() indentation
typical case of "this was looking fine in my editor" :p This is why I like Go with autoformatting :)
arggv leak and gethostname identation fixed in https:/ /github. com/ubuntu/ adsys/pull/ 285.