removed double xcalloc() function; do not leak memory for these small buffers; added safe xstrcpy()

8 files changed, 70 insertions, 41 deletions

diff --git a/libtac/include/libtac.h b/libtac/include/libtac.h
index 8a7381d..7b7518f 100644
--- a/libtac/include/libtac.h
+++ b/libtac/include/libtac.h
@@ -79,7 +79,7 @@ struct areply {
 };
 
 #ifndef TAC_PLUS_MAXSERVERS		
-#define TAC_PLUS_MAXSERVERS 4
+#define TAC_PLUS_MAXSERVERS 8
 #endif
 
 #ifndef TAC_PLUS_PORT
@@ -114,7 +114,7 @@ extern int tac_ver_patch;
 extern int session_id;
 extern int tac_encryption;
 extern const char *tac_secret;
-extern char *tac_login;
+extern char tac_login[64];
 extern int tac_priv_lvl;
 extern int tac_authen_method;
 extern int tac_authen_service;
diff --git a/libtac/lib/acct_s.c b/libtac/lib/acct_s.c
index 200dd62..929378a 100644
--- a/libtac/lib/acct_s.c
+++ b/libtac/lib/acct_s.c
@@ -78,7 +78,7 @@ int tac_acct_send(int fd, int type, const char *user, char *tty,
     tb.flags=(u_char) type;
     tb.authen_method=tac_authen_method;
     tb.priv_lvl=tac_priv_lvl;
-    if (tac_login == NULL) {
+    if (tac_login == NULL || !*tac_login) {
         /* default to PAP */
         tb.authen_type = TAC_PLUS_AUTHEN_TYPE_PAP;
     } else {
diff --git a/libtac/lib/authen_s.c b/libtac/lib/authen_s.c
index 8cb7cb9..87dcb74 100644
--- a/libtac/lib/authen_s.c
+++ b/libtac/lib/authen_s.c
@@ -51,7 +51,7 @@ int tac_authen_send(int fd, const char *user, char *pass, char *tty,
     th=_tac_req_header(TAC_PLUS_AUTHEN, 0);
 
     /* set some header options */
-    if ((tac_login != NULL) && (strcmp(tac_login,"login") == 0)) {
+    if (tac_login != NULL && !strcmp(tac_login,"login")) {
         th->version = TAC_PLUS_VER_0;
     } else {
         th->version = TAC_PLUS_VER_1;
@@ -62,7 +62,7 @@ int tac_authen_send(int fd, const char *user, char *pass, char *tty,
         __FUNCTION__, user, tty, r_addr, \
         (tac_encryption) ? "yes" : "no"))        
         
-    if ((tac_login != NULL) && (strcmp(tac_login,"chap") == 0)) {
+    if (tac_login != NULL && !strcmp(tac_login,"chap")) {
         chal_len = strlen(chal);
         mdp_len = sizeof(u_char) + strlen(pass) + chal_len;
         mdp = (u_char *) xcalloc(1, mdp_len);
@@ -90,13 +90,13 @@ int tac_authen_send(int fd, const char *user, char *pass, char *tty,
     /* fill the body of message */
     tb.action = TAC_PLUS_AUTHEN_LOGIN;
     tb.priv_lvl = tac_priv_lvl;
-    if (tac_login == NULL) {
+    if (tac_login == NULL || !*tac_login) {
         /* default to PAP */
         tb.authen_type = TAC_PLUS_AUTHEN_TYPE_PAP;
     } else {
-        if (strcmp(tac_login,"chap") == 0) {
+        if (!strcmp(tac_login,"chap")) {
             tb.authen_type = TAC_PLUS_AUTHEN_TYPE_CHAP;
-        } else if (strcmp(tac_login,"login") == 0) {
+        } else if (!strcmp(tac_login,"login")) {
             tb.authen_type = TAC_PLUS_AUTHEN_TYPE_ASCII;
         } else {
             tb.authen_type = TAC_PLUS_AUTHEN_TYPE_PAP;
diff --git a/libtac/lib/header.c b/libtac/lib/header.c
index dd04c92..73c4f13 100644
--- a/libtac/lib/header.c
+++ b/libtac/lib/header.c
@@ -33,10 +33,11 @@ int session_id;
 int tac_encryption = 0;
 
 /* Pointer to TACACS+ shared secret string. */
+/* note: tac_secret will point to tacplus_server[i].key */
 const char *tac_secret = NULL;
 
-/* Pointer to TACACS+ shared login string. */
-char *tac_login = NULL; /* default is PAP */
+/* TACACS+ shared login string. */
+char tac_login[64]; /* default is PAP */
 
 /* priv_lvl */
 int tac_priv_lvl = TAC_PLUS_PRIV_LVL_MIN;
diff --git a/libtac/lib/xalloc.c b/libtac/lib/xalloc.c
index ce34c44..d749b52 100644
--- a/libtac/lib/xalloc.c
+++ b/libtac/lib/xalloc.c
@@ -23,7 +23,7 @@
 #include "xalloc.h"
 
 void *xcalloc(size_t nmemb, size_t size) {
-    register void *val = calloc(nmemb, size);
+    void *val = calloc(nmemb, size);
     if(val == 0) {
         TACSYSLOG((LOG_ERR, "%s: calloc(%u,%u) failed", __FUNCTION__,\
             (unsigned) nmemb, (unsigned) size))
@@ -33,7 +33,7 @@ void *xcalloc(size_t nmemb, size_t size) {
 }
 
 void *xrealloc(void *ptr, size_t size) {
-    register void *val = realloc(ptr, size);
+    void *val = realloc(ptr, size);
     if(val == 0) {
         TACSYSLOG((LOG_ERR, "%s: realloc(%u) failed", __FUNCTION__, (unsigned) size))
         exit(1);
diff --git a/pam_tacplus.c b/pam_tacplus.c
index be0c826..635c11d 100644
--- a/pam_tacplus.c
+++ b/pam_tacplus.c
@@ -56,7 +56,7 @@ int _pam_send_account(int tac_fd, int type, const char *user, char *tty,
     struct tac_attrib *attr;
     int retval;
 
-    attr=(struct tac_attrib *)_xcalloc(sizeof(struct tac_attrib));
+    attr=(struct tac_attrib *)xcalloc(1, sizeof(struct tac_attrib));
 
     sprintf(buf, "%lu", (unsigned long)time(NULL));
 
@@ -93,12 +93,17 @@ int _pam_send_account(int tac_fd, int type, const char *user, char *tty,
             __FUNCTION__, 
             tac_acct_flag2str(type),
             task_id);
-        if(re.msg != NULL) free(re.msg);
+
+        if(re.msg != NULL)
+            free(re.msg);
+
         close(tac_fd);
         return -1;
     }
 
-    if(re.msg != NULL) free(re.msg);
+    if(re.msg != NULL)
+        free(re.msg);
+
     close(tac_fd);
     return 0;
 }
diff --git a/support.c b/support.c
index 3181036..7ee2dad 100644
--- a/support.c
+++ b/support.c
@@ -27,28 +27,40 @@
 #include "support.h"
 #include "pam_tacplus.h"
 
+#include <stdlib.h>
+#include <string.h>
+
 tacplus_server_t tac_srv[TAC_PLUS_MAXSERVERS];
 int tac_srv_no = 0;
 
-char *tac_service = NULL;
-char *tac_protocol = NULL;
-char *tac_prompt = NULL;
+char tac_service[64];
+char tac_protocol[64];
+char tac_prompt[64];
 
 /*
-    FIXME using xcalloc() leaks memory for long-running programs that authenticate multiple times
+    safe string copy, like strlcpy() really
 */
-#ifndef xcalloc
-void *_xcalloc (size_t size) {
-    register void *val = calloc (1, size);
-    if (val == 0) {
-        syslog (LOG_ERR, "xcalloc: calloc(1,%u) failed", (unsigned) size);
-        abort();
-    }
-    return val;
+size_t xstrcpy(char *dst, const char *src, size_t dst_size) {
+    if (dst == NULL)
+        _pam_log(LOG_ERR, "xstrcpy(): dst == NULL");
+
+    if (src == NULL)
+        _pam_log(LOG_ERR, "xstrcpy(): src == NULL");
+
+    if (!dst_size)
+        return 0;
+
+    size_t s_len = strlen(src);
+
+    size_t n = s_len;
+    if (n >= dst_size)
+        n = dst_size - 1;
+
+    strncpy(dst, src, n);
+    dst[n] = 0;
+
+    return n;
 }
-#else
-#define _xcalloc xcalloc
-#endif
 
 void _pam_log(int err, const char *format,...) {
     char msg[256];
@@ -191,6 +203,11 @@ int _pam_parse (int argc, const char **argv) {
     memset(tac_srv, 0, sizeof(tacplus_server_t) * TAC_PLUS_MAXSERVERS);
     tac_srv_no = 0;
 
+    tac_service[0] = 0;
+    tac_protocol[0] = 0;
+    tac_prompt[0] = 0;
+    tac_login[0] = 0;
+
     for (ctrl = 0; argc-- > 0; ++argv) {
         if (!strcmp (*argv, "debug")) { /* all */
             ctrl |= PAM_TAC_DEBUG;
@@ -199,14 +216,11 @@ int _pam_parse (int argc, const char **argv) {
         } else if (!strcmp (*argv, "try_first_pass")) { 
             ctrl |= PAM_TAC_TRY_FIRST_PASS;
         } else if (!strncmp (*argv, "service=", 8)) { /* author & acct */
-            tac_service = (char *) _xcalloc (strlen (*argv + 8) + 1);
-            strcpy (tac_service, *argv + 8);
+            xstrcpy (tac_service, *argv + 8, sizeof(tac_service));
         } else if (!strncmp (*argv, "protocol=", 9)) { /* author & acct */
-            tac_protocol = (char *) _xcalloc (strlen (*argv + 9) + 1);
-            strcpy (tac_protocol, *argv + 9);
+            xstrcpy (tac_protocol, *argv + 9, sizeof(tac_protocol));
         } else if (!strncmp (*argv, "prompt=", 7)) { /* authentication */
-            tac_prompt = (char *) _xcalloc (strlen (*argv + 7) + 1);
-            strcpy (tac_prompt, *argv + 7);
+            xstrcpy (tac_prompt, *argv + 7, sizeof(tac_prompt));
             /* Replace _ with space */
             int chr;
             for (chr = 0; chr < strlen(tac_prompt); chr++) {
@@ -214,6 +228,8 @@ int _pam_parse (int argc, const char **argv) {
                     tac_prompt[chr] = ' ';
                 }
             }
+        } else if (!strncmp (*argv, "login=", 6)) {
+            xstrcpy (tac_login, *argv + 6, sizeof(tac_login));
         } else if (!strcmp (*argv, "acct_all")) {
             ctrl |= PAM_TAC_ACCT;
         } else if (!strncmp (*argv, "server=", 7)) { /* authen & acct */
@@ -265,10 +281,11 @@ int _pam_parse (int argc, const char **argv) {
                 tac_srv[i].key = current_secret;
             }
         } else if (!strncmp (*argv, "timeout=", 8)) {
+            /* FIXME atoi() doesn't handle invalid numeric strings well */
             tac_timeout = atoi(*argv + 8);
-        } else if (!strncmp (*argv, "login=", 6)) {
-            tac_login = (char *) _xcalloc (strlen (*argv + 6) + 1);
-            strcpy (tac_login, *argv + 6);
+
+            if (tac_timeout < 0)
+                tac_timeout = 0;
         } else {
             _pam_log (LOG_WARNING, "unrecognized option: %s", *argv);
         }
@@ -282,6 +299,11 @@ int _pam_parse (int argc, const char **argv) {
         for(n = 0; n < tac_srv_no; n++) {
             _pam_log(LOG_DEBUG, "server[%d] { addr=%s, key='%s' }", n, tac_ntop(tac_srv[n].addr->ai_addr), tac_srv[n].key);
         }
+
+        _pam_log(LOG_DEBUG, "tac_service='%s'", tac_service);
+        _pam_log(LOG_DEBUG, "tac_protocol='%s'", tac_protocol);
+        _pam_log(LOG_DEBUG, "tac_prompt='%s'", tac_prompt);
+        _pam_log(LOG_DEBUG, "tac_login='%s'", tac_login);
     }
 
     return ctrl;
diff --git a/support.h b/support.h
index 260dea6..9cbd040 100644
--- a/support.h
+++ b/support.h
@@ -34,8 +34,9 @@ typedef struct {
 extern tacplus_server_t tac_srv[TAC_PLUS_MAXSERVERS];
 extern int tac_srv_no;
 
-extern char *tac_service;
-extern char *tac_protocol;
+extern char tac_service[64];
+extern char tac_protocol[64];
+extern char tac_prompt[64];
 
 int _pam_parse (int, const char **);
 unsigned long _resolve_name (char *);