From 662310ddb0a24af4fc7fc240d0664d1d5c42a19a Mon Sep 17 00:00:00 2001 From: Walter de Jong Date: Wed, 6 Aug 2014 16:42:32 +0200 Subject: protect against buffer overflow --- src/pam_radius_auth.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'src') diff --git a/src/pam_radius_auth.c b/src/pam_radius_auth.c index 887ee1e..853b8a1 100644 --- a/src/pam_radius_auth.c +++ b/src/pam_radius_auth.c @@ -107,6 +107,12 @@ static int _pam_parse(int argc, CONST char **argv, radius_conf_t *conf) /* generic options */ if (!strncmp(*argv,"conf=",5)) { + /* protect against buffer overflow */ + if (strlen(*argv+5) >= sizeof(conf_file)) { + _pam_log(LOG_ERR, "conf= argument too long"); + conf_file[0] = 0; + return 0; + } strcpy(conf_file,*argv+5); } else if (!strcmp(*argv, "use_first_pass")) { -- cgit v1.2.3 From d4d97e242d878b03314ef679dd14eeedc6c6ce3c Mon Sep 17 00:00:00 2001 From: Walter de Jong Date: Wed, 6 Aug 2014 16:45:38 +0200 Subject: only print debug messages when configured --- src/pam_radius_auth.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/pam_radius_auth.c b/src/pam_radius_auth.c index 853b8a1..27d9b2f 100644 --- a/src/pam_radius_auth.c +++ b/src/pam_radius_auth.c @@ -60,11 +60,12 @@ #include "pam_radius_auth.h" -#define DPRINT if (ctrl & PAM_DEBUG_ARG) _pam_log +#define DPRINT if (opt_debug & PAM_DEBUG_ARG) _pam_log /* internal data */ static CONST char *pam_module_name = "pam_radius_auth"; static char conf_file[BUFFER_SIZE]; /* configuration file */ +static int opt_debug = FALSE; /* print debug info */ /* we need to save these from open_session to close_session, since * when close_session will be called we won't be root anymore and @@ -145,6 +146,7 @@ static int _pam_parse(int argc, CONST char **argv, radius_conf_t *conf) } else if (!strcmp(*argv, "debug")) { ctrl |= PAM_DEBUG_ARG; conf->debug = 1; + opt_debug = TRUE; } else { _pam_log(LOG_WARNING, "unrecognized option '%s'", *argv); @@ -259,7 +261,6 @@ static uint32_t get_ipaddr(char *host) { static int host2server(radius_server_t *server) { char *p; - int ctrl = 1; /* for DPRINT */ if ((p = strchr(server->hostname, ':')) != NULL) { *(p++) = '\0'; /* split the port off from the host name */ -- cgit v1.2.3 From b5552f23be2531ad4edd647be342ebef6b13b81d Mon Sep 17 00:00:00 2001 From: Walter de Jong Date: Wed, 6 Aug 2014 16:47:12 +0200 Subject: fix wrong format string modifier --- src/pam_radius_auth.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/pam_radius_auth.c b/src/pam_radius_auth.c index 27d9b2f..a88acae 100644 --- a/src/pam_radius_auth.c +++ b/src/pam_radius_auth.c @@ -289,15 +289,15 @@ static int host2server(radius_server_t *server) if (p) { /* maybe it's not "radius" */ svp = getservbyname (p, "udp"); /* quotes allow distinction from above, lest p be radius or radacct */ - DPRINT(LOG_DEBUG, "DEBUG: getservbyname('%s', udp) returned %d.\n", p, svp); + DPRINT(LOG_DEBUG, "DEBUG: getservbyname('%s', udp) returned %p.\n", p, svp); *(--p) = ':'; /* be sure to put the delimiter back */ } else { if (!server->accounting) { svp = getservbyname ("radius", "udp"); - DPRINT(LOG_DEBUG, "DEBUG: getservbyname(radius, udp) returned %d.\n", svp); + DPRINT(LOG_DEBUG, "DEBUG: getservbyname(radius, udp) returned %p.\n", svp); } else { svp = getservbyname ("radacct", "udp"); - DPRINT(LOG_DEBUG, "DEBUG: getservbyname(radacct, udp) returned %d.\n", svp); + DPRINT(LOG_DEBUG, "DEBUG: getservbyname(radacct, udp) returned %p.\n", svp); } } -- cgit v1.2.3 From b061e094c70f139e78f8a7803d11f7f76ade6c75 Mon Sep 17 00:00:00 2001 From: Walter de Jong Date: Wed, 6 Aug 2014 16:49:15 +0200 Subject: use DPRINT for logging debug messages --- src/pam_radius_auth.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'src') diff --git a/src/pam_radius_auth.c b/src/pam_radius_auth.c index a88acae..5ef44e0 100644 --- a/src/pam_radius_auth.c +++ b/src/pam_radius_auth.c @@ -1222,9 +1222,7 @@ error: } } - if (ctrl & PAM_DEBUG_ARG) { - _pam_log(LOG_DEBUG, "authentication %s", retval==PAM_SUCCESS ? "succeeded":"failed"); - } + DPRINT(LOG_DEBUG, "authentication %s", retval==PAM_SUCCESS ? "succeeded":"failed"); close(config.sockfd); cleanup(config.server); -- cgit v1.2.3 From ee0e16001c4073f770a134e7e18e89d3d2b9d7b4 Mon Sep 17 00:00:00 2001 From: Walter de Jong Date: Wed, 6 Aug 2014 16:55:57 +0200 Subject: clear the response Just making sure, it seems cleaner to make sure that the function can not exit with the response in some random/undefined state --- src/pam_radius_auth.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src') diff --git a/src/pam_radius_auth.c b/src/pam_radius_auth.c index 5ef44e0..c4b9fd1 100644 --- a/src/pam_radius_auth.c +++ b/src/pam_radius_auth.c @@ -779,6 +779,8 @@ static int talk_radius(radius_conf_t *conf, AUTH_HDR *request, AUTH_HDR *respons /* loop over all available servers */ while (server != NULL) { + /* clear the response */ + memset(response, 0, sizeof(AUTH_HDR)); /* only look up IP information as necessary */ if ((retval = host2server(server)) != PAM_SUCCESS) { -- cgit v1.2.3 From 7a91c66ccd03a2b47a98b37c2ed917b312e067f1 Mon Sep 17 00:00:00 2001 From: Walter de Jong Date: Thu, 14 Aug 2014 15:41:13 +0200 Subject: no semicolons here (empty statements) --- src/pam_radius_auth.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/pam_radius_auth.c b/src/pam_radius_auth.c index c4b9fd1..f4e8fab 100644 --- a/src/pam_radius_auth.c +++ b/src/pam_radius_auth.c @@ -1086,8 +1086,8 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh,int flags,int argc,CONST c DPRINT(LOG_DEBUG, "Username now %s from ruser", user); } else { DPRINT(LOG_DEBUG, "Skipping ruser for non-root auth"); - }; - }; + } + } /* * Get the IP address of the authentication server -- cgit v1.2.3 From cd4a5e245622acb4871e97b554b1b38d26f393ec Mon Sep 17 00:00:00 2001 From: Walter de Jong Date: Fri, 15 Aug 2014 12:13:40 +0200 Subject: code style consistency --- src/pam_radius_auth.c | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) (limited to 'src') diff --git a/src/pam_radius_auth.c b/src/pam_radius_auth.c index f4e8fab..7fdc0aa 100644 --- a/src/pam_radius_auth.c +++ b/src/pam_radius_auth.c @@ -186,27 +186,27 @@ static uint32_t ipstr2long(char *ip_str) { *ptr = '\0'; while(*ip_str != '.' && *ip_str != '\0' && count < 4) { - if(!isdigit(*ip_str)) { - return((uint32_t)0); + if (!isdigit(*ip_str)) { + return (uint32_t)0; } *ptr++ = *ip_str++; count++; } - if(count >= 4 || count == 0) { - return((uint32_t)0); + if (count >= 4 || count == 0) { + return (uint32_t)0; } *ptr = '\0'; cur_byte = atoi(buf); - if(cur_byte < 0 || cur_byte > 255) { - return ((uint32_t)0); + if (cur_byte < 0 || cur_byte > 255) { + return (uint32_t)0; } ip_str++; ipaddr = ipaddr << 8 | (uint32_t)cur_byte; } - return(ipaddr); + return ipaddr; } /* @@ -219,23 +219,23 @@ static int good_ipaddr(char *addr) { dot_count = 0; digit_count = 0; while(*addr != '\0' && *addr != ' ') { - if(*addr == '.') { + if (*addr == '.') { dot_count++; digit_count = 0; - } else if(!isdigit(*addr)) { + } else if (!isdigit(*addr)) { dot_count = 5; } else { digit_count++; - if(digit_count > 3) { + if (digit_count > 3) { dot_count = 5; } } addr++; } - if(dot_count != 3) { - return(-1); + if (dot_count != 3) { + return -1; } else { - return(0); + return 0; } } @@ -246,13 +246,13 @@ static int good_ipaddr(char *addr) { static uint32_t get_ipaddr(char *host) { struct hostent *hp; - if(good_ipaddr(host) == 0) { - return(ipstr2long(host)); - } else if((hp = gethostbyname(host)) == (struct hostent *)NULL) { - return((uint32_t)0); + if (good_ipaddr(host) == 0) { + return ipstr2long(host); + } else if ((hp = gethostbyname(host)) == (struct hostent *)NULL) { + return (uint32_t)0; } - return(ntohl(*(uint32_t *)hp->h_addr)); + return ntohl(*(uint32_t *)hp->h_addr); } /* @@ -1118,7 +1118,7 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh,int flags,int argc,CONST c retval = pam_get_item(pamh, PAM_AUTHTOK, (CONST void **) &password); PAM_FAIL_CHECK; - if(password) { + if (password) { password = strdup(password); DPRINT(LOG_DEBUG, "Got password %s", password); } @@ -1403,12 +1403,12 @@ PAM_EXTERN int pam_sm_chauthtok(pam_handle_t *pamh, int flags, int argc, CONST c /* grab the old password (if any) from the previous password layer */ retval = pam_get_item(pamh, PAM_OLDAUTHTOK, (CONST void **) &password); PAM_FAIL_CHECK; - if(password) password = strdup(password); + if (password) password = strdup(password); /* grab the new password (if any) from the previous password layer */ retval = pam_get_item(pamh, PAM_AUTHTOK, (CONST void **) &new_password); PAM_FAIL_CHECK; - if(new_password) new_password = strdup(new_password); + if (new_password) new_password = strdup(new_password); /* preliminary password change checks. */ if (flags & PAM_PRELIM_CHECK) { -- cgit v1.2.3 From 0bd64858aca06a5eeed4fe04a0fa6ba9f5248104 Mon Sep 17 00:00:00 2001 From: Walter de Jong Date: Fri, 15 Aug 2014 14:32:24 +0200 Subject: check return value of rad_converse() --- src/pam_radius_auth.c | 1 + 1 file changed, 1 insertion(+) (limited to 'src') diff --git a/src/pam_radius_auth.c b/src/pam_radius_auth.c index 7fdc0aa..3f2436e 100644 --- a/src/pam_radius_auth.c +++ b/src/pam_radius_auth.c @@ -1196,6 +1196,7 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh,int flags,int argc,CONST c /* It's full challenge-response, we should have echo on */ retval = rad_converse(pamh, PAM_PROMPT_ECHO_ON, challenge, &resp2challenge); + PAM_FAIL_CHECK; /* now that we've got a response, build a new radius packet */ build_radius_packet(request, user, resp2challenge, &config); -- cgit v1.2.3