Nmap Development mailing list archives
Patches to NMAP-6.01
From: Bill Parker <wp02855 () gmail com>
Date: Tue, 13 Nov 2012 12:14:32 -0800
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/
Current thread:
- Patches to NMAP-6.01 Bill Parker (Nov 13)
- Re: Patches to NMAP-6.01 David Fifield (Nov 20)
- Re: Patches to NMAP-6.01 Bill Parker (Nov 22)
- Re: Patches to NMAP-6.01 David Fifield (Nov 20)