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>

diff --git a/chat/chat.c b/chat/chat.c
index f625a8a4fee521fdc3e6e13edc88dd3463675af6..0740229163dd8ca54f2c1be2d23bcab6ffaad6e6 100644 (file)
--- 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 d27f6306c4b03f2e8cabf3a93de37751c50e15bb..202d557a5a4b41539ee1065ad43fbacb1f3bc5bb 100644 (file)
--- 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 d1e0cf80212c265cdfeae2c4c6659d15c8422108..6b2a0261c2c8141e01912419b77cd153b01ddad0 100644 (file)
--- 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 40f08b3c361d32649388aa7423b635ec2985d54d..70154664432e8e90904d901bf311aa6511036a57 100644 (file)
--- 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 795f8a9901ad5ae203daa053308502143fa8f9fa..a36b1d942be4d530215078aebba7699156ebc75a 100644 (file)
--- 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 f2ff59d85380cbeb339910628aff9ff706e94c8d..214adaeca455dcb5638f87d4c541e6300136f748 100644 (file)
--- 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 207e5bf2da167c0cfd35d5ba63e17dbb4be7cb73..39d3dbdf4cab33c45434464413fe8c7a6ebfe926 100644 (file)
--- 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 ee9d343835b8a83995d15648f9ba10e2d8483f45..c7ace96f18a43112699e5644aac65a760c799f9e 100644 (file)
--- 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 b4bc896bce4941f91056e11b9b1c39ecc7a21d7c..11cf27cfdd9dc023750d62685b3c5c5fe181f8e4 100644 (file)
--- 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 a0dde005fdaa960c77eb19be86ea1e2728f380b5..ab923cc114a7114cb79928454719c04431e31e2f 100644 (file)
--- 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 aa5fececc17d2cd9a21d00c50c2eec14c8204a49..c5811107d215f3a8d4ecac74559380ca602bac9b 100644 (file)
--- 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;
   }