oss-sec mailing list archives
Re: pam_timestamp internals
From: "Dmitry V. Levin" <ldv () altlinux org>
Date: Mon, 31 Mar 2014 15:37:02 +0400
On Mon, Mar 31, 2014 at 12:57:11PM +0200, Sebastian Krahmer wrote:
On Mon, Mar 31, 2014 at 02:32:09PM +0400, Dmitry V. Levin wrote:On Mon, Mar 24, 2014 at 01:46:43PM +0100, Sebastian Krahmer wrote:When playing with some PAM modules for my own projects, I came across some implications of pam_timestamp (which is part of upstream linux-pam) that should probably be addressed. Most importantly, there seems to be a path traversal issue:Thanks, Sebastian! The issue has been fixed in upstream linux-pam by commit https://git.fedorahosted.org/cgit/linux-pam.git/commit/?id=Linux-PAM-1_1_8-32-g9dcead8Thanks for taking care. I was about to write a patch on my own, but seems not necessary anymore. However, I think that + if (!strlen(tty) || !strcmp(tty, ".") || !strcmp(tty, "..")) { could be insufficient.
There is a code in check_tty() that handles '/': if (strchr(tty, '/') != NULL) { ... tty = strrchr(tty, '/') + 1; }
Any occurence of "." inside tty name should be evil.
Strange - yes, but why evil?
Above strcmp() matches exactly "." or "..", but you also want "../../" etc which should pass above check.
After commit 9dcead8, check_tty() handles all such cases.
For the ruser check, the strchr(ruser, '/') safes this, but ".." occurence may also be treatened appropriately.
Commit 9dcead8 also makes get_ruser() reject "." and ".." as invalid ruser values. -- ldv
Attachment:
_bin
Description:
Current thread:
- pam_timestamp internals Sebastian Krahmer (Mar 24)
- Re: pam_timestamp internals cve-assign (Mar 26)
- Re: pam_timestamp internals Dmitry V. Levin (Mar 31)
- Re: pam_timestamp internals Sebastian Krahmer (Mar 31)
- Re: pam_timestamp internals Dmitry V. Levin (Mar 31)
- Re: pam_timestamp internals Sebastian Krahmer (Mar 31)
- Re: pam_timestamp internals Sebastian Krahmer (Mar 31)