Patches to NMAP-6.01

[Bill Parker <wp02855 () gmail com>]

Ok, lets try this again :)

Hello,

   In reviewing code in NMAP-6.01, I found various calls to
malloc()/calloc()/realloc() which were not being checked
for a return value of NULL which would indicate failure.

In addition, I also found deprecated calls to bzero() which
were replaced by memset() which is ANSI/ISO compliant, a
call to read() which isn't checked for failure, and added
a warning message in the event the call to munmap() cannot
unmap the memory location in question.

In directory nmap-6.01/libpcap, file 'pcap-sita.c', several
of these calls were found, and code was added to check
for return values of NULL.  Additionally, the deprecated
calls to bzero() were replaced with memset().  The patch file
listing is below:

--- pcap-sita.c.orig    2012-11-11 13:55:56.121999898 -0800
+++ pcap-sita.c 2012-11-11 14:07:00.570002622 -0800
@@ -311,9 +311,13 @@

        if (u->serv_addr == NULL) {
                u->serv_addr = malloc(sizeof(struct sockaddr_in));
+               if (u->serv_addr == NULL) { /* oops, malloc() failed... */
+                       fprintf(stderr, "warning: unable to allocate memory
for u->serv_addr.\n");
+                       return 0;
+               }
        }
        ip = u->ip;
-       bzero((char *)u->serv_addr, sizeof(struct sockaddr_in));
+       memset((char *)u->serv_addr, 0, sizeof(struct sockaddr_in));
        u->serv_addr->sin_family                = AF_INET;
        u->serv_addr->sin_addr.s_addr   = inet_addr(ip);
        u->serv_addr->sin_port                  = htons(IOP_SNIFFER_PORT);
@@ -417,11 +421,19 @@
        int                     IOPportnum = 0;

        iface = malloc(sizeof(iface_t));                /* get memory for a
structure */
-       bzero((char *)iface, sizeof(iface_t));
+       if (iface == NULL) { /* oops, malloc() failed... */
+               fprintf(stderr, "warning: unable to allocate memory for
interface.\n");
+               return NULL;
+       }
+       memset((char *)iface, 0, sizeof(iface_t));      /* bzero()
deprecated, replaced with memset() */

        iface->iftype = iftype;                                 /* remember
the interface type of this interface */

        name = malloc(strlen(IOPname) + 1);             /* get memory for
the IOP's name */
+       if (name == NULL) { /* oops, malloc() failed... */
+               fprintf(stderr, "warning: unable to allocate memory for
IOPname.\n");
+               return NULL;
+       }
        strcpy(name, IOPname);                                  /* and copy
it in */
        iface->IOPname = name;                                  /* and
stick it into the structure */

@@ -447,6 +459,10 @@

        sprintf(buf, "%s_%s", proto, port);             /* compose the
user's name for that IOP port name */
        name = malloc(strlen(buf) + 1);                 /* get memory for
that name */
+       if (name == NULL) { /* oops, malloc() failed */
+               fprintf(stderr, "warning: unable to allocate memory for IOP
port name.\n");
+               return NULL;
+       }
        strcpy(name, buf);                                              /*
and copy it in */
        iface->name = name;                                             /*
and stick it into the structure */

@@ -548,7 +564,7 @@
                                        snprintf(errbuf, PCAP_ERRBUF_SIZE,
"malloc: %s", pcap_strerror(errno));
                                        return -1;
                                }
-                               bzero((char *)iff, sizeof(pcap_if_t));
+                               memset((char *)iff, 0, sizeof(pcap_if_t));
     /* bzero() deprecated, replaced with memset() */
                                if (acn_if_list == 0)   acn_if_list = iff;
                                     /* remember the head of the list */
                                if (prev_iff)
prev_iff->next = iff;                           /* insert a forward link */

@@ -588,7 +604,7 @@
                                                snprintf(errbuf,
PCAP_ERRBUF_SIZE, "malloc: %s", pcap_strerror(errno));
                                                return -1;
                                        }
-                                       bzero((char *)addr,
sizeof(pcap_addr_t));
+                                       memset((char *)addr, 0,
sizeof(pcap_addr_t));   /* bzero() deprecated, replaced with memset() */
                                        if (iff->addresses == 0)
iff->addresses = addr;
                                        if (prev_addr) prev_addr->next =
addr;                                                  /* insert a forward
link */
                                        if (*ptr) {/* if there is a count
for the address */
@@ -596,7 +612,7 @@
                                                        snprintf(errbuf,
PCAP_ERRBUF_SIZE, "malloc: %s", pcap_strerror(errno));
                                                        return -1;
                                                }
-                                               bzero((char *)s,
sizeof(struct sockaddr_in));
+                                               memset((char *)s, 0,
sizeof(struct sockaddr_in)); /* bzero() deprecated, replaced with memset()
*/
                                                addr->addr = (struct
sockaddr *)s;
                                                s->sin_family           =
AF_INET;
                                                s->sin_addr.s_addr      =
*(bpf_u_int32 *)(ptr + 1);                    /* copy the address in */
@@ -608,7 +624,7 @@
                                                        snprintf(errbuf,
PCAP_ERRBUF_SIZE, "malloc: %s", pcap_strerror(errno));
                                                        return -1;
                                                }
-                                               bzero((char *)s,
sizeof(struct sockaddr_in));
+                                               memset((char *)s, 0,
sizeof(struct sockaddr_in)); /* bzero() deprecated, replaced with memset()
*/
                                                addr->netmask = (struct
sockaddr *)s;
                                                s->sin_family           =
AF_INET;
                                                s->sin_addr.s_addr      =
*(bpf_u_int32*)(ptr + 1);
@@ -620,7 +636,7 @@
                                                        snprintf(errbuf,
PCAP_ERRBUF_SIZE, "malloc: %s", pcap_strerror(errno));
                                                        return -1;
                                                }
-                                               bzero((char *)s,
sizeof(struct sockaddr_in));
+                                               memset((char *)s, 0,
sizeof(struct sockaddr_in)); /* bzero() deprecated, replaced with memset()
*/
                                                addr->broadaddr = (struct
sockaddr *)s;
                                                s->sin_family           =
AF_INET;
                                                s->sin_addr.s_addr      =
*(bpf_u_int32*)(ptr + 1);
@@ -632,7 +648,7 @@
                                                        snprintf(errbuf,
PCAP_ERRBUF_SIZE, "malloc: %s", pcap_strerror(errno));
                                                        return -1;
                                                }
-                                               bzero((char *)s,
sizeof(struct sockaddr_in));
+                                               memset((char *)s, 0,
sizeof(struct sockaddr_in)); /* bzero() deprecated, replaced with memset()
*/
                                                addr->dstaddr = (struct
sockaddr *)s;
                                                s->sin_family           =
AF_INET;
                                                s->sin_addr.s_addr      =
*(bpf_u_int32*)(ptr + 1);

In directory nmap-6.01/libpcap, file 'pcap.c', no checks are made
for calls to malloc() returning a NULL value, indicating failure.
The patch file listing is below:

--- pcap.c.orig 2012-11-11 14:09:10.976000492 -0800
+++ pcap.c      2012-11-11 14:13:16.188000996 -0800
@@ -1528,6 +1528,10 @@
                            strlen(pcap_version_string);
                        full_pcap_version_string =
                            malloc(full_pcap_version_string_len);
+                       if (full_pcap_version_string == NULL) { /* oops,
malloc() failed */
+                               fprintf(stderr, "warning: unable to
allocate memory for pcap_version_string.\n");
+                               return NULL;
+                       }
                        sprintf(full_pcap_version_string,
                            pcap_version_string_fmt, wpcap_version_string,
                            pcap_version_string);
@@ -1545,7 +1549,10 @@
                            strlen(packet_version_string) +
                            strlen(pcap_version_string);
                        full_pcap_version_string =
malloc(full_pcap_version_string_len);
-
+                       if (full_pcap_version_string == NULL) { /* oops,
malloc() failed */
+                               fprintf(stderr, "warning: unable to
allocate memory for pcap_version_string.\n");
+                               return NULL;
+                       }
                        sprintf(full_pcap_version_string,
                            pcap_version_string_packet_dll_fmt,
                            wpcap_version_string, packet_version_string,
@@ -1574,6 +1581,10 @@
                    sizeof dospfx + strlen(pcap_version_string);
                full_pcap_version_string =
                    malloc(full_pcap_version_string_len);
+               if (full_pcap_version_string == NULL) { /* oops, malloc()
failed */
+                       fprintf(stderr, "warning: unable to allocate memory
for pcap version string.\n");
+                       return NULL;
+               }
                strcpy(full_pcap_version_string, dospfx);
                strcat(full_pcap_version_string, pcap_version_string);
        }
In directory nmap-6.01/libpcap, file 'pcap-bpf.c', the call to
bzero() was replaced by memset() as bzero is deprecated.  The
patch file listing is below:

--- pcap-bpf.c.orig     2012-11-11 14:41:20.475000614 -0800
+++ pcap-bpf.c  2012-11-11 14:41:48.289999912 -0800
@@ -1665,7 +1665,7 @@
                            pcap_strerror(errno));
                        goto bad;
                }
-               bzero(&bz, sizeof(bz));
+               memset(&bz, 0, sizeof(bz)); /* bzero() deprecated, replaced
by memset() */
                bz.bz_bufa = p->md.zbuf1;
                bz.bz_bufb = p->md.zbuf2;
                bz.bz_buflen = p->md.zbufsize;
In directory nmap-6.01/libdnet-stripped/src, file 'arp-win32.c',
no check is made for malloc() returning a NULL value, indicating
failure.  The patch file listing is below:

--- arp-win32.c.orig    2012-11-11 13:21:39.610997814 -0800
+++ arp-win32.c 2012-11-11 13:22:50.305998646 -0800
@@ -108,6 +108,10 @@
                if (arp->iptable)
                        free(arp->iptable);
                arp->iptable = malloc(len);
+               if (arp->iptable == NULL && len > 0) { /* oops, malloc()
failed, print warning  */
+                       fprintf(stderr, "Unable to allocate memory for
arp->iptable in function arp_loop...\n");
+                       return -1;
+               }
                ret = GetIpNetTable(arp->iptable, &len, FALSE);
                if (ret == NO_ERROR)
                        break;

The issues above were seen in libpcap-1.3.0, and I submitted the
patch files to incorporate these fixes via github in the
mcr/libpcap master branch (the pull requests have the username
'dogbert2' which belongs to me on github.com).
In directory nmap-6.01/libdnet-stripped/src, file 'intf-win32.c',
no check is made for calloc() returning a NULL value, indicating
failure.  The patch file listing is below:

--- intf-win32.c.orig   2012-11-11 13:25:06.880000641 -0800
+++ intf-win32.c        2012-11-11 14:19:01.204999036 -0800
@@ -94,9 +94,13 @@
                        ifc->max *= 2;
                        ifc->idx = realloc(ifc->idx,
                            sizeof(ifc->idx[0]) * ifc->max);
+                       if (ifc->idx == NULL) /* realloc() failed */
+                               return; /* might want to print warning msg
to user */
                } else {
                        ifc->max = 8;
                        ifc->idx = malloc(sizeof(ifc->idx[0]) * ifc->max);
+                       if (ifc->idx == NULL) /* malloc() failed */
+                               return; /* might want to print warning msg
to user */
                }
        }
        ifc->idx[ifc->cnt].ipv4 = ipv4_idx;
In directory nmap-6.01/libdnet-stripped/src, file 'intf.c', the
last argument in getsockname should be casted to (socklen_t *)
per the manual/posix specification.  The patch file listing is
below:

--- intf.c.orig 2012-11-11 13:38:48.479003304 -0800
+++ intf.c      2012-11-11 13:39:23.294000369 -0800
@@ -832,7 +832,7 @@
                return (-1);

        n = sizeof(sin);
-       if (getsockname(intf->fd, (struct sockaddr *)&sin, &n) < 0)
+       if (getsockname(intf->fd, (struct sockaddr *)&sin, (socklen_t *)&n)
< 0)
                return (-1);

        addr_ston((struct sockaddr *)&sin, &entry->intf_addr);
In directory nmap-6.01/libdnet-stripped/src, file 'intf.c', the
last argument in getsockopt should be casted to (socklen_t *)
per the manual/posix specification:

--- ip.c.orig   2012-11-11 13:37:24.548005127 -0800
+++ ip.c        2012-11-11 13:38:16.674995419 -0800
@@ -41,7 +41,7 @@
 #endif
 #ifdef SO_SNDBUF
        len = sizeof(n);
-       if (getsockopt(i->fd, SOL_SOCKET, SO_SNDBUF, &n, &len) < 0)
+       if (getsockopt(i->fd, SOL_SOCKET, SO_SNDBUF, &n, (socklen_t *)&len)
< 0)
                return (ip_close(i));

        for (n += 128; n < 1048576; n += 128) {
In directory nmap-6.01/libdnet-stripped/src, file 'rand.c', the
call to read() is not checked for a failure condition when
trying to read from /dev/urandom.  The patch file below adds a
warning message to the end user.

--- rand.c.orig 2012-11-11 13:31:39.401993081 -0800
+++ rand.c      2012-11-11 13:33:46.507002603 -0800
@@ -22,6 +22,7 @@
 # include <sys/time.h>
 # include <unistd.h>
 #endif
+#include <stdio.h>
 #include <fcntl.h>
 #include <stdlib.h>
 #include <string.h>
@@ -77,11 +78,13 @@
        CryptReleaseContext(hcrypt, 0);
 #else
        struct timeval *tv = (struct timeval *)seed;
-       int fd;
+       int fd, r_stat;

        if ((fd = open("/dev/arandom", O_RDONLY)) != -1 ||
            (fd = open("/dev/urandom", O_RDONLY)) != -1) {
-               read(fd, seed + sizeof(*tv), sizeof(seed) - sizeof(*tv));
+               r_stat = read(fd, seed + sizeof(*tv), sizeof(seed) -
sizeof(*tv));
+               if (r_stat < 0) /* read failed, let user know */
+                       fprintf(stderr, "failure on read() from
/dev/arandom or /dev/urandom\n");
                close(fd);
        }
        gettimeofday(tv, NULL);
In directory nmap-6.01/libdnet-stripped/src, file 'route-win32.c',
I found calls to malloc() which were not checked for a return value
of NULL, which would indicate failure.  The patch file below corrects
this issue:

--- route-win32.c.orig  2012-11-11 13:35:15.231996335 -0800
+++ route-win32.c       2012-11-11 14:24:13.229001771 -0800
@@ -35,6 +35,10 @@
        route_t *r;

        r = calloc(1, sizeof(route_t));
+       if (r == NULL) { /* oops, calloc() has failed */
+               fprintf(stderr, "warning: unable to allocate memory in
function route_open.\n");
+               return r;
+       }
        r->iphlpapi = GetModuleHandle("iphlpapi.dll");

        return r;
@@ -133,6 +137,10 @@
                if (r->ipftable)
                        free(r->ipftable);
                r->ipftable = malloc(len);
+               if (r->ipftable == NULL && len > 0) {
+                       fprintf(stderr, "Unable to allocate memory for
r->ipftable in function route_loop...\n");
+                       return (-1);
+               }
                ret = GetIpForwardTable(r->ipftable, &len, FALSE);
                if (ret == NO_ERROR)
                        break;
I have submitted the above patches to Dug Song (author of libdnet)
via email, but have not received any response from him (though the
email did not bounce back).

In directory nmap-6.01/liblinear, file 'predict.c', I found calls
to malloc() made with no check for a return value of NULL, which
would indicate failure.  The patch file below corrects this problem.

--- predict.c.orig      2012-11-11 13:42:20.828000539 -0800
+++ predict.c   2012-11-11 13:46:21.495998129 -0800
@@ -63,8 +63,16 @@
                }

                labels=(int *) malloc(nr_class*sizeof(int));
+               if (labels == NULL) { /* oops, malloc() failed, print
warning and bail */
+                       fprintf(stderr, "warning: unable to allocate memory
for nr_class.\n");
+                       exit(1);
+               }
                get_labels(model_,labels);
                prob_estimates = (double *) malloc(nr_class*sizeof(double));
+               if (prob_estimates == NULL) { /* oops, malloc() failed,
print warning and bail */
+                       fprintf(stderr, "warning: unable to allocate memory
for nr_class.\n");
+                       exit(1);
+               }
                fprintf(output,"labels");
                for(j=0;j<nr_class;j++)
                        fprintf(output," %d",labels[j]);
@@ -207,6 +215,10 @@
        }

        x = (struct feature_node *) malloc(max_nr_attr*sizeof(struct
feature_node));
+       if (x == NULL) { /* oops, malloc() failed, print warning and bail */
+               fprintf(stderr, "warning: unable to allocate memory for
max_nr_attr.\n");
+               exit(1);
+       }
        do_predict(input, output, model_);
        free_and_destroy_model(&model_);
        free(line);
In directory nmap-6.01, file 'nmap.cc', a call to munmap() is made,
but in the event of a error condition, the user receives no warning
that a memory segment could not be unmapped.  While it is not possible
to recover from a munmap() failure, the patch file below at least
gives the end-user information about which memory segment could not
be unmapped.

--- nmap.cc.orig        2012-11-11 14:29:17.768998606 -0800
+++ nmap.cc     2012-11-11 14:33:34.357999981 -0800
@@ -2257,7 +2257,17 @@

   /* Ensure the log file ends with a newline */
   filestr[filelen - 1] = '\n';
-  munmap(filestr, filelen);
+
+  /*
+   * we cannot recover from a munmap() failure, but
+   * we can print out an informative message with
+   * the address location and size of the memory
+   * mapping which wasn't released by munmap().
+   */
+
+  if (munmap(filestr, filelen) != 0)
+       error("Warning: unable to unmap memory at address %p and size
%u.\n", filestr, filelen);
+
   return 0;
 }

All patch files will be attached to this email for review.

Please feel free to modify/change any of the patches above
to suit the needs of nmap development.

If the above work is usable by the nmap developers, the
only thing I would like is credit for finding and patching
the reported issues above.

Bill Parker (wp02855 at gmail.com)

Attachment: rand.c.patch
Description:

Attachment: arp-win32.c.patch
Description:

Attachment: intf.c.patch
Description:

Attachment: intf-win32.c.patch
Description:

Attachment: pcap-sita.c.patch
Description:

Attachment: route-win32.c.patch
Description:

Attachment: ip.c.patch
Description:

Attachment: nmap.cc.patch
Description:

Attachment: pcap.c.patch
Description:

Attachment: pcap-bpf.c.patch
Description:

Attachment: predict.c.patch
Description:

_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://seclists.org/nmap-dev/