#27: pam_timestamp path traversal issue -----------------------+------------------------------ Reporter: tmraz | Owner: pam-developers@… Type: security | Status: new Priority: major | Component: modules Version: | Keywords: Blocked By: | Blocking: -----------------------+------------------------------ pam_timestamp uses PAM_RUSER and PAM_TTY directly without any checks. If the user can mainpulate PAM_RUSER and PAM_TTY contents (which is mostly not possible, but there might be scenarios where it is) he would be able to get access to a service without proper checking.
See http://seclists.org/oss-sec/2014/q1/645 for the original report by Sebastian Krahmer.
I suppose sufficient mitigation would be to look for ".." in both PAM_RUSER and PAM_TTY and reject authentication attempt if they contain this string.
I would also document that the module should never be used with services where the authenticating user can manipulate the PAM_RUSER variable contents.
#27: pam_timestamp path traversal issue ---------------------+------------------------------- Reporter: tmraz | Owner: pam-developers@… Type: security | Status: new Priority: major | Component: modules Version: | Resolution: Keywords: | Blocked By: Blocking: | ---------------------+-------------------------------
Comment (by ldv):
To put thing more formally, if either PAM_RUSER or PAM_TTY equals to "." or "..", or starts with "../", or ends with "/..", or contains "/../", then get_timestamp_name should return PAM_AUTH_ERR.
#27: pam_timestamp path traversal issue ---------------------+------------------------------- Reporter: tmraz | Owner: pam-developers@… Type: security | Status: new Priority: major | Component: modules Version: | Resolution: Keywords: | Blocked By: Blocking: | ---------------------+-------------------------------
Comment (by tmraz):
Actually the tty cannot contain any '/' as that is already checked in check_tty().
#27: pam_timestamp path traversal issue ---------------------+------------------------------- Reporter: tmraz | Owner: pam-developers@… Type: security | Status: new Priority: major | Component: modules Version: | Resolution: Keywords: | Blocked By: Blocking: | ---------------------+-------------------------------
Comment (by ldv):
Since check_tty() already strips '/', only two tty values should be disallowed: "." and "..". With regards to ruser value, besides "." and "..", it seems reasonable to disallow any value containing '/', to avoid potential DoS attacks (e.g. specially crafted ruser would result to creation of a directory in place of a regular timestamp file).
pam_timestamp uses values of PAM_RUSER and PAM_TTY as components of the timestamp pathname it creates, so extra care should be taken to avoid potential directory traversal issues.
* modules/pam_timestamp/pam_timestamp.c (check_tty): Treat "." and ".." tty values as invalid. (get_ruser): Treat "." and ".." ruser values, as well as any ruser value containing '/', as invalid.
Fixes CVE-2014-2583.
Reported-by: Sebastian Krahmer krahmer@suse.de --- modules/pam_timestamp/pam_timestamp.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/modules/pam_timestamp/pam_timestamp.c b/modules/pam_timestamp/pam_timestamp.c index 5193733..b3f08b1 100644 --- a/modules/pam_timestamp/pam_timestamp.c +++ b/modules/pam_timestamp/pam_timestamp.c @@ -158,7 +158,7 @@ check_tty(const char *tty) tty = strrchr(tty, '/') + 1; } /* Make sure the tty wasn't actually a directory (no basename). */ - if (strlen(tty) == 0) { + if (!strlen(tty) || !strcmp(tty, ".") || !strcmp(tty, "..")) { return NULL; } return tty; @@ -243,6 +243,17 @@ get_ruser(pam_handle_t *pamh, char *ruserbuf, size_t ruserbuflen) if (pwd != NULL) { ruser = pwd->pw_name; } + } else { + /* + * This ruser is used by format_timestamp_name as a component + * of constructed timestamp pathname, so ".", "..", and '/' + * are disallowed to avoid potential path traversal issues. + */ + if (!strcmp(ruser, ".") || + !strcmp(ruser, "..") || + strchr(ruser, '/')) { + ruser = NULL; + } } if (ruser == NULL || strlen(ruser) >= ruserbuflen) { *ruserbuf = '\0';
On Čt, 2014-03-27 at 02:44 +0400, Dmitry V. Levin wrote:
pam_timestamp uses values of PAM_RUSER and PAM_TTY as components of the timestamp pathname it creates, so extra care should be taken to avoid potential directory traversal issues.
- modules/pam_timestamp/pam_timestamp.c (check_tty): Treat
"." and ".." tty values as invalid. (get_ruser): Treat "." and ".." ruser values, as well as any ruser value containing '/', as invalid.
Fixes CVE-2014-2583.
Reported-by: Sebastian Krahmer krahmer@suse.de
modules/pam_timestamp/pam_timestamp.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/modules/pam_timestamp/pam_timestamp.c b/modules/pam_timestamp/pam_timestamp.c index 5193733..b3f08b1 100644 --- a/modules/pam_timestamp/pam_timestamp.c +++ b/modules/pam_timestamp/pam_timestamp.c @@ -158,7 +158,7 @@ check_tty(const char *tty) tty = strrchr(tty, '/') + 1; } /* Make sure the tty wasn't actually a directory (no basename). */
- if (strlen(tty) == 0) {
- if (!strlen(tty) || !strcmp(tty, ".") || !strcmp(tty, "..")) { return NULL; } return tty;
@@ -243,6 +243,17 @@ get_ruser(pam_handle_t *pamh, char *ruserbuf, size_t ruserbuflen) if (pwd != NULL) { ruser = pwd->pw_name; }
- } else {
/*
* This ruser is used by format_timestamp_name as a component
* of constructed timestamp pathname, so ".", "..", and '/'
* are disallowed to avoid potential path traversal issues.
*/
if (!strcmp(ruser, ".") ||
!strcmp(ruser, "..") ||
strchr(ruser, '/')) {
ruser = NULL;
} if (ruser == NULL || strlen(ruser) >= ruserbuflen) { *ruserbuf = '\0';}
This looks fine to me. Please commit.
#27: pam_timestamp path traversal issue ---------------------+------------------------------- Reporter: tmraz | Owner: pam-developers@… Type: security | Status: closed Priority: major | Component: modules Version: | Resolution: fixed Keywords: | Blocked By: Blocking: | ---------------------+------------------------------- Changes (by ldv):
* resolution: => fixed * status: new => closed
Comment:
Just for the record, the issue was fixed long time ago with commit https://git.fedorahosted.org/cgit/linux-pam.git/commit/?id=Linux-PAM- 1_1_8-32-g9dcead8
pam-developers@lists.fedorahosted.org