[patch] fix strncpy() management

[Vasily Kulikov <segoon () openwall com>]

The patch fixes several strncpy() calls to properly set '\0' in the end
of the string.  The bugs were found with the help of the following
coccinelle script:

    @@
    expression d, s, n;
    statement S;
    @@

    (
    strncpy(d, s, n);
    d[n-1] = \( 0 \| '\0' \) ;
    |
    strncpy((char*)d, s, n);
    d[n-1] = \( 0 \| '\0' \) ;
    |
    strncpy(d, s, sizeof(d)-1);
    d[sizeof(d)-1] = \( 0 \| '\0' \) ;
    |
    strncpy(d, s, n);
    - d[n] 
    + d[n-1]
       = \( 0 \| '\0' \) ;
    |
    strncpy(d, s, n);
    + d[n-1] = 0;
    )

All found cases were manually checked, some of them were false
positives.  Some code doesn't need = '\0', but needs changes to the size
argument.

strncpy() from libpcap/ need additional review as it looks like several
strncats deliberately don't add zero at the end of the string to satisfy
setsockopt/getsockopt API.
--

 FPEngine.cc           |    1 +
 idle_scan.cc          |    1 +
 libnetutil/netutil.cc |    2 ++
 libpcre/pcreposix.c   |    2 +-
 ncat/ncat_connect.c   |    1 +
 ncat/ncat_main.c      |    2 ++
 nping/NpingOps.cc     |    1 +
 nping/NpingTargets.cc |    4 +++-
 nping/ProbeMode.cc    |    1 +
 nping/utils_net.cc    |    6 ++++--
 nse_pcrelib.cc        |    1 +
 output.cc             |    4 ++++
 tcpip.cc              |    2 ++
 13 files changed, 24 insertions(+), 4 deletions(-)

--
Index: output.cc
===================================================================
--- output.cc   (revision 31756)
+++ output.cc   (working copy)
@@ -1973,6 +1973,7 @@
     time_t lastboot;
     lastboot = (time_t) currenths->seq.lastboot;
     strncpy(tmbuf, ctime(&lastboot), sizeof(tmbuf));
+    tmbuf[sizeof(tmbuf)-1] = 0;
     chomp(tmbuf);
     gettimeofday(&tv, NULL);
     if (o.verbose)
@@ -2102,6 +2103,7 @@
         if (!hostname_tbl[i][0]) {
           numhostnames++;
           strncpy(&hostname_tbl[i][0], sd.hostname, sizeof(hostname_tbl[i]));
+          hostname_tbl[i][sizeof(hostname_tbl[i])-1] = 0;
           break;
         }
       }
@@ -2115,6 +2117,7 @@
         if (!ostype_tbl[i][0]) {
           numostypes++;
           strncpy(&ostype_tbl[i][0], sd.ostype, sizeof(ostype_tbl[i]));
+          ostype_tbl[i][sizeof(ostype_tbl[i])-1] = 0;
           break;
         }
       }
@@ -2128,6 +2131,7 @@
         if (!devicetype_tbl[i][0]) {
           numdevicetypes++;
           strncpy(&devicetype_tbl[i][0], sd.devicetype, sizeof(devicetype_tbl[i]));
+          devicetype_tbl[i][sizeof(devicetype_tbl[i])-1] = 0;
           break;
         }
       }
Index: tcpip.cc
===================================================================
--- tcpip.cc    (revision 31756)
+++ tcpip.cc    (working copy)
@@ -1253,6 +1253,7 @@
   realfrag = htons(ntohs(ip->ip_off) & IP_OFFMASK);
   tot_len = htons(ip->ip_len);
   strncpy(sourcehost, inet_ntoa(bullshit), 16);
+  sourcehost[15] = 0;
   i = 4 * (ntohs(ip->ip_hl) + ntohs(tcp->th_off));
   if (ip->ip_p == IPPROTO_TCP) {
     if (realfrag)
@@ -1329,6 +1330,7 @@
   realfrag = htons(ntohs(ip->ip_off) & IP_OFFMASK);
   tot_len = htons(ip->ip_len);
   strncpy(sourcehost, inet_ntoa(bullshit), 16);
+  sourcehost[15] = 0;
   i = 4 * (ntohs(ip->ip_hl)) + 8;
   if (ip->ip_p == IPPROTO_UDP) {
     if (realfrag)
Index: nse_pcrelib.cc
===================================================================
--- nse_pcrelib.cc      (revision 31756)
+++ nse_pcrelib.cc      (working copy)
@@ -92,6 +92,7 @@
                luaL_error(L, "cannot set locale");
 
        strncpy(old_locale, setlocale(LC_CTYPE, NULL), 255); /* store the locale */
+       old_locale[255] = 0;
 
        if(setlocale(LC_CTYPE, locale) == NULL)        /* set new locale */
                luaL_error(L, "cannot set locale");
Index: ncat/ncat_connect.c
===================================================================
--- ncat/ncat_connect.c (revision 31756)
+++ ncat/ncat_connect.c (working copy)
@@ -565,6 +565,7 @@
 
                 srcaddr.un.sun_family = AF_UNIX;
                 strncpy(srcaddr.un.sun_path, tmp_name, sizeof(srcaddr.un.sun_path));
+                srcaddr.un.sun_path[sizeof(srcaddr.un.sun_path) - 1] = 0;
                 free (tmp_name);
             }
             nsi_set_localaddr(cs.sock_nsi, &srcaddr.storage, SUN_LEN((struct sockaddr_un *)&srcaddr.storage));
Index: ncat/ncat_main.c
===================================================================
--- ncat/ncat_main.c    (revision 31756)
+++ ncat/ncat_main.c    (working copy)
@@ -705,6 +705,7 @@
             if (o.proto == IPPROTO_UDP) {
                 srcaddr.un.sun_family = AF_UNIX;
                 strncpy(srcaddr.un.sun_path, source, sizeof(srcaddr.un.sun_path));
+                srcaddr.un.sun_path[sizeof(srcaddr.un.sun_path) - 1] = 0;
                 srcaddrlen = SUN_LEN(&srcaddr.un);
             }
             else
@@ -740,6 +741,7 @@
             memset(&targetss.storage, 0, sizeof(struct sockaddr_un));
             targetss.un.sun_family = AF_UNIX;
             strncpy(targetss.un.sun_path, argv[optind], sizeof(targetss.un.sun_path));
+            targetss.un.sun_path[sizeof(targetss.un.sun_path) - 1] = 0;
             targetsslen = SUN_LEN(&targetss.un);
             o.target = argv[optind];
             optind++;
Index: libnetutil/netutil.cc
===================================================================
--- libnetutil/netutil.cc       (revision 31756)
+++ libnetutil/netutil.cc       (working copy)
@@ -1626,7 +1626,9 @@
       char gwbuf[INET6_ADDRSTRLEN];
 
       strncpy(destbuf, inet_ntop_ez(&dcrn->routes[i].dest, sizeof(dcrn->routes[i].dest)), sizeof(destbuf));
+      destbuf[sizeof(destbuf)-1] = 0;
       strncpy(gwbuf, inet_ntop_ez(&dcrn->routes[i].gw, sizeof(dcrn->routes[i].gw)), sizeof(gwbuf));
+      gwbuf[sizeof(gwbuf)-1] = 0;
       /*
       netutil_error("WARNING: Unable to find appropriate interface for system route to %s/%u gw %s",
         destbuf, dcrn->routes[i].netmask_bits, gwbuf);
Index: libpcre/pcreposix.c
===================================================================
--- libpcre/pcreposix.c (revision 31756)
+++ libpcre/pcreposix.c (working copy)
@@ -173,7 +173,7 @@
 
 addmessage = " at offset ";
 addlength = (preg != NULL && (int)preg->re_erroffset != -1)?
-  strlen(addmessage) + 6 : 0;
+  strlen(addmessage) + 7 : 0;
 
 if (errbuf_size > 0)
   {
Index: FPEngine.cc
===================================================================
--- FPEngine.cc (revision 31756)
+++ FPEngine.cc (working copy)
@@ -2463,6 +2463,7 @@
   this->link_eth = true;
   if (devname != NULL) {
     strncpy(this->eth_hdr.devname, devname, sizeof(this->eth_hdr.devname)-1);
+    this->eth_hdr.devname[sizeof(this->eth_hdr.devname)-1] = 0;
     if ((this->eth_hdr.ethsd = eth_open_cached(devname)) == NULL)
       fatal("%s: Failed to open ethernet device (%s)", __func__, devname);
   } else {
Index: nping/NpingTargets.cc
===================================================================
--- nping/NpingTargets.cc       (revision 31756)
+++ nping/NpingTargets.cc       (working copy)
@@ -222,8 +222,10 @@
   memcpy( t, &next, sizeof( struct sockaddr_storage ) );
   /* If current spec is a named host (not a range), store name in supplied buff */
   if(current_group.get_namedhost()){
-    if( hname!=NULL && hlen>0 )
+    if( hname!=NULL && hlen>0 ) {
         strncpy(hname, specs[ current_spec ], hlen);  
+        hname[hlen-1] = 0;
+    }
   }else{ /* If current spec is not a named host, insert NULL in the first position */
     if( hname!=NULL && hlen>0 )
         hname[0]='\0';
Index: nping/utils_net.cc
===================================================================
--- nping/utils_net.cc  (revision 31756)
+++ nping/utils_net.cc  (working copy)
@@ -620,7 +620,7 @@
                
       /* Store info */
          memset(&(archive[current_index]), 0, sizeof(cached_host_t) );
-         strncpy(archive[current_index].hostname, host, MAX_CACHED_HOSTNAME_LEN);
+         strncpy(archive[current_index].hostname, host, MAX_CACHED_HOSTNAME_LEN-1);
          archive[current_index].sslen = *sslen;
          memcpy(&(archive[current_index].ss), ss, *sslen);
 
@@ -712,7 +712,7 @@
         
       /* Store the hostent entry in the cache */   
          memset(&(archive[current_index]), 0, sizeof(gethostbynamecached_t) );
-         strncpy(archive[current_index].hostname, host, MAX_CACHED_HOSTNAME_LEN);
+         strncpy(archive[current_index].hostname, host, MAX_CACHED_HOSTNAME_LEN-1);
          archive[current_index].h = hostentcpy( result );
 
       /* Return the entry that we've just added */
@@ -1057,6 +1057,7 @@
   }
 
   strncpy((char*)dstbuff, protoinfo, dstlen);
+  dstbuff[dstlen-1] = 0;
 
   return OP_SUCCESS;
   
@@ -1129,6 +1130,7 @@
   }
 
   strncpy((char*)dstbuff, protoinfo, dstlen);
+  dstbuff[dstlen-1] = 0;
 
   return OP_SUCCESS;
   
Index: nping/ProbeMode.cc
===================================================================
--- nping/ProbeMode.cc  (revision 31756)
+++ nping/ProbeMode.cc  (working copy)
@@ -1336,6 +1336,7 @@
         strncpy(filterstring, buffer, sizeof(filterstring)-1);
     else
         strncpy(filterstring, "", 2);
+    filterstring[sizeof(filterstring)-1] = 0;
     nping_print(DBG_1, "BPF-filter: %s", filterstring);
     return filterstring;
  }
Index: nping/NpingOps.cc
===================================================================
--- nping/NpingOps.cc   (revision 31756)
+++ nping/NpingOps.cc   (working copy)
@@ -2248,6 +2248,7 @@
 
 int NpingOps::setEchoPassphrase(const char *str){
   strncpy(this->echo_passphrase, str, sizeof(echo_passphrase)-1);
+  this->echo_passphrase[sizeof(echo_passphrase) - 1] = 0;
   this->echo_passphrase_set=true;
   return OP_SUCCESS;
 } /* End of setEchoPassphrase() */
Index: idle_scan.cc
===================================================================
--- idle_scan.cc        (revision 31756)
+++ idle_scan.cc        (working copy)
@@ -1046,6 +1046,7 @@
   if (!*lastproxy) {
     initialize_idleproxy(&proxy, proxyName, target->v4hostip(), ports);
     strncpy(lastproxy, proxyName, sizeof(lastproxy));
+    lastproxy[sizeof(lastproxy)-1] = 0;
   }
 
   /* If we don't have timing infoz for the new target, we'll use values
-- 
Vasily Kulikov
http://www.openwall.com - bringing security into open computing environments
_______________________________________________
Sent through the dev mailing list
http://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/