Fix several issues uncovered by Coverity (#397)

* Fix for coverity issue 436265, we should cap copy to size of destination buffer

Signed-off-by: Eivind Næss <eivnaes@yahoo.com>

* Fix for coverity issue 436262, llv6_ntoa() returns a pointer to a buffer that can be up to 64 bytes long; likely not a problem, but this will quiet coverity

Signed-off-by: Eivind Næss <eivnaes@yahoo.com>

* Fix for coverity issue 436251, not freeing path in the normal flow of the code

Signed-off-by: Eivind Næss <eivnaes@yahoo.com>

* Fixing coverity issue #436258, Digest maybe uninitialized in some paths of this code

Signed-off-by: Eivind Næss <eivnaes@yahoo.com>

* Fix for coverity issue 436254, forgot to free 's' before returning from the function?

Signed-off-by: Eivind Næss <eivnaes@yahoo.com>

* Fixing coverity issue #436251, memory leak in put_string() function

Signed-off-by: Eivind Næss <eivnaes@yahoo.com>

* Fixing coverity issue 436215, should copy at most sizeof(devname) bytes

Signed-off-by: Eivind Næss <eivnaes@yahoo.com>

* Fixing coverity issue #436203, if no authentication (or no accounting) server was found, we still need to free the allocated local instance

Signed-off-by: Eivind Næss <eivnaes@yahoo.com>

* Fixing coverity issue #436171, use of uninitialized variable

Signed-off-by: Eivind Næss <eivnaes@yahoo.com>

* Use of signed vs unsigned variable in printf for MD4Update

Signed-off-by: Eivind Næss <eivnaes@yahoo.com>

* Fixing coverity issue #436182, fixing possible buffer overrun in handling of PW_CLASS attribute

Signed-off-by: Eivind Næss <eivnaes@yahoo.com>

* Fixing coverity issue #436156

Signed-off-by: Eivind Næss <eivnaes@yahoo.com>

* Compile errors

Signed-off-by: Eivind Næss <eivnaes@yahoo.com>

[paulus@ozlabs.org - Squashed to avoid breaking bisection]

Signed-off-by: Eivind Næss <eivnaes@yahoo.com>
Signed-off-by: Paul Mackerras <paulus@ozlabs.org>

11 files changed, 26 insertions, 11 deletions

diff --git a/chat/chat.c b/chat/chat.c
index f625a8a..0740229 100644
--- a/chat/chat.c
+++ b/chat/chat.c
@@ -1133,7 +1133,7 @@ int chat_send (register char *s)
 
 	if (verbose)
 	    msgf("timeout set to %d seconds", timeout);
-
+	free(s);
 	return 0;
     }
 
@@ -1247,8 +1247,11 @@ int write_char(int c)
 
 int put_string(register char *s)
 {
+    char *s1;
     quiet = 0;
+
     s = clean(s, 1);
+    s1 = s;
 
     if (verbose) {
 	if (quiet)
@@ -1263,8 +1266,10 @@ int put_string(register char *s)
 	register char c = *s++;
 
 	if (c != '\\') {
-	    if (!write_char (c))
+	    if (!write_char (c)) {
+		free(s1);
 		return 0;
+	    }
 	    continue;
 	}
 
@@ -1283,8 +1288,10 @@ int put_string(register char *s)
 	    break;
 
 	default:
-	    if (!write_char (c))
+	    if (!write_char (c)) {
+		free(s1);
 		return 0;
+	    }
 	    break;
 	}
 	checksigs();
@@ -1292,6 +1299,7 @@ int put_string(register char *s)
 
     alarm(0);
     alarmed = 0;
+    free(s1);
     return (1);
 }
 
diff --git a/pppd/auth.c b/pppd/auth.c
index d27f630..202d557 100644
--- a/pppd/auth.c
+++ b/pppd/auth.c
@@ -1240,7 +1240,7 @@ np_finished(int unit, int proto)
 static void
 check_maxoctets(void *arg)
 {
-    unsigned int used;
+    unsigned int used = 0;
     ppp_link_stats_st stats;
 
     if (ppp_get_link_stats(&stats)) {
diff --git a/pppd/chap_ms.c b/pppd/chap_ms.c
index d1e0cf8..6b2a026 100644
--- a/pppd/chap_ms.c
+++ b/pppd/chap_ms.c
@@ -686,7 +686,7 @@ GenerateAuthenticatorResponse(unsigned char* PasswordHashHash,
 
     int		i;
     PPP_MD_CTX *ctx;
-    u_char	Digest[SHA_DIGEST_LENGTH];
+    u_char	Digest[SHA_DIGEST_LENGTH] = {};
     int     hash_len;
     u_char	Challenge[8];
 
diff --git a/pppd/eap.c b/pppd/eap.c
index 40f08b3..7015466 100644
--- a/pppd/eap.c
+++ b/pppd/eap.c
@@ -2654,7 +2654,7 @@ eap_response(eap_state *esp, u_char *inp, int id, int len)
 				char tmp[MAXNAMELEN+1];
 
 				strcpy(tmp, strrchr(rhostname, '\\') + 1);
-				strcpy(rhostname, tmp);
+				strlcpy(rhostname, tmp, sizeof(rhostname));
 			}
 
 			if (chap_verify_hook)
diff --git a/pppd/ipv6cp.c b/pppd/ipv6cp.c
index 795f8a9..a36b1d9 100644
--- a/pppd/ipv6cp.c
+++ b/pppd/ipv6cp.c
@@ -1490,7 +1490,7 @@ ipv6cp_script_done(void *arg)
 static void
 ipv6cp_script(char *script)
 {
-    char strspeed[32], strlocal[32], strremote[32];
+    char strspeed[32], strlocal[64], strremote[64];
     char *argv[8];
 
     sprintf(strspeed, "%d", baud_rate);
diff --git a/pppd/options.c b/pppd/options.c
index f2ff59d..214adae 100644
--- a/pppd/options.c
+++ b/pppd/options.c
@@ -1795,6 +1795,8 @@ loadplugin(char **argv)
     }
     info("Plugin %s loaded.", arg);
     (*init)();
+    if (path != arg)
+	free(path);
     return 1;
 
  errclose:
diff --git a/pppd/plugins/pppoatm/pppoatm.c b/pppd/plugins/pppoatm/pppoatm.c
index 207e5bf..39d3dbd 100644
--- a/pppd/plugins/pppoatm/pppoatm.c
+++ b/pppd/plugins/pppoatm/pppoatm.c
@@ -92,7 +92,7 @@ static int setdevname_pppoatm(const char *cp, const char **argv, int doit)
 		return 1;
 
 	memcpy(&pvcaddr, &addr, sizeof pvcaddr);
-	strlcpy(devnam, cp, MAXPATHLEN);
+	strlcpy(devnam, cp, sizeof(devnam));
 	ppp_set_devnam(devnam);
 	devstat.st_mode = S_IFSOCK;
 	if (the_channel != &pppoa_channel) {
diff --git a/pppd/plugins/pppoe/plugin.c b/pppd/plugins/pppoe/plugin.c
index ee9d343..c7ace96 100644
--- a/pppd/plugins/pppoe/plugin.c
+++ b/pppd/plugins/pppoe/plugin.c
@@ -389,7 +389,7 @@ PPPoEDevnameHook(char *cmd, char **argv, int doit)
     /* Close socket */
     close(fd);
     if (r && doit) {
-	strlcpy(devnam, cmd, MAXPATHLEN);
+	strlcpy(devnam, cmd, sizeof(devnam));
 	if (the_channel != &pppoe_channel) {
 
 	    the_channel = &pppoe_channel;
diff --git a/pppd/plugins/radius/radius.c b/pppd/plugins/radius/radius.c
index b4bc896..11cf27c 100644
--- a/pppd/plugins/radius/radius.c
+++ b/pppd/plugins/radius/radius.c
@@ -649,7 +649,8 @@ radius_setparams(VALUE_PAIR *vp, char *msg, REQUEST_INFO *req_info,
                 break;
 	    case PW_CLASS:
 		/* Save Class attribute to pass it in accounting request */
-		if (vp->lvalue <= MAXCLASSLEN) {
+		// if (vp->lvalue <= MAXCLASSLEN) { // <- Attribute could be this big, but vp->strvalue is limited to AUTH_STRING_LEN characters
+		if (vp->lvalue <= AUTH_STRING_LEN) {
 		    rstate.class_len=vp->lvalue;
 		    memcpy(rstate.class, vp->strvalue, rstate.class_len);
 		} /* else too big for our buffer - ignore it */
diff --git a/pppd/plugins/radius/radrealms.c b/pppd/plugins/radius/radrealms.c
index a0dde00..ab923cc 100644
--- a/pppd/plugins/radius/radrealms.c
+++ b/pppd/plugins/radius/radrealms.c
@@ -148,9 +148,13 @@ lookup_realm(char const *user,
 
     if (accts->max)
 	*acctserver = accts;
+    else
+	free(accts);
 
     if (auths->max)
 	*authserver = auths;
+    else
+	free(auths);
 
     return;
 }
diff --git a/pppd/ppp-md4.c b/pppd/ppp-md4.c
index aa5fece..c581110 100644
--- a/pppd/ppp-md4.c
+++ b/pppd/ppp-md4.c
@@ -326,7 +326,7 @@ MD4Update(MD4_CTX *MDp, unsigned char *X, unsigned int count)
   }
   else if (count > 512) /* Check for count too large */
   {
-    printf("\nError: MD4Update called with illegal count value %d.",
+    printf("\nError: MD4Update called with illegal count value %u.",
 	   count);
     return;
   }