Nmap Development mailing list archives
Re: Parfait analysis of nmap 6.25
From: David Fifield <david () bamsoftware com>
Date: Fri, 21 Dec 2012 22:21:28 -0800
On Mon, Dec 10, 2012 at 08:10:15AM -0800, Rich Burridge wrote:
I'm in the process of updating the version of nmap that's in the Oracle Solaris O/S to 6.25. Part of this update requires a security audit, including running the Parfait static code analysis tool on the nmap code.
Thank you for the test results. I have gone through them and made code changes for some. The changes have already been applied to trunk. I attached a patch for the same changes against 6.25.
Error: Null pointer dereference (CWE 476) Read from null pointer 'fdn' at line 328 of components/nmap/build/amd64/ncat/ncat_core.c in function 'blocking_fdinfo_send'. Function 'get_fdinfo' may return constant 'NULL' at line 615, called at line 366 in function 'ncat_broadcast'. Constant 'NULL' passed into function 'blocking_fdinfo_send', argument 'fdn', from call at line 367. Null pointer introduced at line 615 of components/nmap/build/amd64/ncat/util.c in function 'get_fdinfo'. at line 330 of components/nmap/build/amd64/ncat/ncat_core.c in function 'blocking_fdinfo_send'. Function 'get_fdinfo' may return constant 'NULL' at line 615, called at line 366 in function 'ncat_broadcast'. Constant 'NULL' passed into function 'blocking_fdinfo_send', argument 'fdn', from call at line 367. Null pointer introduced at line 615 of components/nmap/build/amd64/ncat/util.c in function 'get_fdinfo'. Error: Null pointer dereference (CWE 476) Read from null pointer 'fdn' at line 946 of components/nmap/build/amd64/ncat/ncat_listen.c in function 'shutdown_sockets'. Function 'get_fdinfo' may return constant 'NULL' at line 615, called at line 945. Null pointer introduced at line 615 of components/nmap/build/amd64/ncat/util.c in function 'get_fdinfo'.
This dereference couldn't happen, I think, if other parts of the code work correctly, but I added some additional asserts to make sure.
Error: File Leak File Descriptor Leak: Leaked File Descriptor s at line 522 of components/nmap/build/amd64/ncat/ncat_proxy.c in function 'handle_connect'. s initialized at line 450 with Socket s leaks when socket_errno() != 0 at line 478.
This looks like a false positive to me. The socket is always closed when the while (!socket_errno() || socket_errno() == EINTR) loop finishes. I don't see a way for the function to return without closing.
Error: Null pointer dereference (CWE 476) Read from null pointer '((int*)&Target::v4hostip(tpreq->targ)->S_un)' at line 544 of components/nmap/build/amd64/nmap_dns.cc in function 'process_result(unsigned int, char*, int, unsigned short)'. Function 'Target::v4hostip() const' may return constant 'NULL' at line 285, called at line 544. Null pointer introduced at line 285 of components/nmap/build/amd64/Target.cc in function 'Target::v4hostip() const'. Error: Null pointer dereference (CWE 476) Read from null pointer '((int*)&Target::v4hostip((*hostI))->S_un)' at line 1189 of components/nmap/build/amd64/nmap_dns.cc in function 'nmap_mass_rdns_core(Target**, int)'. Function 'Target::v4hostip() const' may return constant 'NULL' at line 285, called at line 1189. Null pointer introduced at line 285 of components/nmap/build/amd64/Target.cc in function 'Target::v4hostip() const'. Error: Null pointer dereference (CWE 476) Read from null pointer '((int*)&Target::v4hostip(hss->target)->S_un)' at line 2112 of components/nmap/build/amd64/osscan2.cc in function 'HostOsScan::send_closedudp_probe(HostOsScanStats*, int, unsigned short, unsigned short)'. Function 'Target::v4hostip() const' may return constant 'NULL' at line 285, called at line 2112. Null pointer introduced at line 285 of components/nmap/build/amd64/Target.cc in function 'Target::v4hostip() const'. Error: Null pointer dereference (CWE 476) Read from null pointer '((int*)&Target::v4hostip((*std::_List_iterator<HostOsScanInfo*>::operator*(&hostI))->target)->S_un)' at line 3383 of components/nmap/build/amd64/osscan2.cc in function 'OsScanInfo::findIncompleteHost(sockaddr_storage*)'. Function 'Target::v4hostip() const' may return constant 'NULL' at line 285, called at line 3383. Null pointer introduced at line 285 of components/nmap/build/amd64/Target.cc in function 'Target::v4hostip() const'.
The v4hostip method returns non-NULL when the target has an IPv4 address, which is the case in all these places. It's perhaps not immediately clear that this is the case, and the protections against a non-IPv4 address sneaking in are perhaps not as strong as they could be (for example, OSScan::os_scan splits up IPv4 and IPv6 targets, but I don't know if the address family is enforced at a lower level), but I think it's fine to leave these alone.
Error: Buffer overrun Read outside array bounds (CWE 125): In pointer dereference of ipids[(i - 1)] with index '(i - 1)' Pointer size is 6 elements (of 4 bytes each), index is 29 at line 232 of components/nmap/build/amd64/osscan2.cc in function 'get_ipid_sequence(int, int*, int)'. called at line 504 of components/nmap/build/amd64/idle_scan.cc in function 'initialize_idleproxy(idle_proxy_info*, char*, in_addr const*, scan_lists const*)' with ipids = ipids. at line 235 of components/nmap/build/amd64/osscan2.cc in function 'get_ipid_sequence(int, int*, int)'. called at line 504 of components/nmap/build/amd64/idle_scan.cc in function 'initialize_idleproxy(idle_proxy_info*, char*, in_addr const*, scan_lists const*)' with ipids = ipids. at line 236 of components/nmap/build/amd64/osscan2.cc in function 'get_ipid_sequence(int, int*, int)'. called at line 504 of components/nmap/build/amd64/idle_scan.cc in function 'initialize_idleproxy(idle_proxy_info*, char*, in_addr const*, scan_lists const*)' with ipids = ipids. at line 238 of components/nmap/build/amd64/osscan2.cc in function 'get_ipid_sequence(int, int*, int)'. called at line 504 of components/nmap/build/amd64/idle_scan.cc in function 'initialize_idleproxy(idle_proxy_info*, char*, in_addr const*, scan_lists const*)' with ipids = ipids. Error: Buffer overrun Read outside array bounds (CWE 125): In pointer dereference of ipids[i] with index 'i' Pointer size is 6 elements (of 4 bytes each), index is 30 at line 232 of components/nmap/build/amd64/osscan2.cc in function 'get_ipid_sequence(int, int*, int)'. called at line 504 of components/nmap/build/amd64/idle_scan.cc in function 'initialize_idleproxy(idle_proxy_info*, char*, in_addr const*, scan_lists const*)' with ipids = ipids. at line 235 of components/nmap/build/amd64/osscan2.cc in function 'get_ipid_sequence(int, int*, int)'. called at line 504 of components/nmap/build/amd64/idle_scan.cc in function 'initialize_idleproxy(idle_proxy_info*, char*, in_addr const*, scan_lists const*)' with ipids = ipids. at line 236 of components/nmap/build/amd64/osscan2.cc in function 'get_ipid_sequence(int, int*, int)'. called at line 504 of components/nmap/build/amd64/idle_scan.cc in function 'initialize_idleproxy(idle_proxy_info*, char*, in_addr const*, scan_lists const*)' with ipids = ipids. at line 238 of components/nmap/build/amd64/osscan2.cc in function 'get_ipid_sequence(int, int*, int)'. called at line 504 of components/nmap/build/amd64/idle_scan.cc in function 'initialize_idleproxy(idle_proxy_info*, char*, in_addr const*, scan_lists const*)' with ipids = ipids.
These I believe are false positives. Parfait may think that the index can be as big as 32 because of the check u16 ipid_diffs[32]; assert(numSamples < (int) (sizeof(ipid_diffs) / 2)); but it also will be no larger than the numSamples passed as an argument, which is smaller.
Error: Null pointer dereference (CWE 476) Read from null pointer 'hsi' at line 459 of components/nmap/build/amd64/osscan2.cc in function 'doSeqTests(OsScanInfo*, HostOsScan*)'. Function 'OsScanInfo::nextIncompleteHost()' may return constant 'NULL' at line 3398, called at line 458. Null pointer introduced at line 3398 in function 'OsScanInfo::nextIncompleteHost()'. Error: Null pointer dereference (CWE 476) Read from null pointer 'hsi' at line 629 of components/nmap/build/amd64/osscan2.cc in function 'doTUITests(OsScanInfo*, HostOsScan*)'. Function 'OsScanInfo::nextIncompleteHost()' may return constant 'NULL' at line 3398, called at line 628. Null pointer introduced at line 3398 in function 'OsScanInfo::nextIncompleteHost()'.
These look okay to me. The list of incomplete hosts is checked to be nonempty in both places. The code could be more clear but I think it's safe.
Error: Null pointer dereference (CWE 476) Read from null pointer 'port' at line 344 of components/nmap/build/amd64/portlist.cc in function 'PortList::setServiceProbeResults(unsigned short, int, serviceprobestate, char const*, service_tunnel_type, char const*, char const*, char const*, char const*, char const*, char const*, std::vector<char const*, std::allocator<char const*> > const*, char const*)'. Function 'PortList::createPort(unsigned short, unsigned char)' may return constant 'NULL' at line 671, called at line 343. Null pointer introduced at line 671 in function 'PortList::createPort(unsigned short, unsigned char)'. Error: Null pointer dereference (CWE 476) Write to null pointer 'current' at line 520 of components/nmap/build/amd64/portlist.cc in function 'PortList::setPortState(unsigned short, unsigned char, int)'. Function 'PortList::createPort(unsigned short, unsigned char)' may return constant 'NULL' at line 671, called at line 518. Null pointer introduced at line 671 in function 'PortList::createPort(unsigned short, unsigned char)'. Error: Null pointer dereference (CWE 476) Write to null pointer 'answer' at line 880 of components/nmap/build/amd64/portlist.cc in function 'PortList::setStateReason(unsigned short, unsigned char, unsigned short, unsigned char, sockaddr_storage const*)'. Function 'PortList::createPort(unsigned short, unsigned char)' may return constant 'NULL' at line 671, called at line 877. Null pointer introduced at line 671 in function 'PortList::createPort(unsigned short, unsigned char)'. at line 885 of components/nmap/build/amd64/portlist.cc in function 'PortList::setStateReason(unsigned short, unsigned char, unsigned short, unsigned char, sockaddr_storage const*)'. Function 'PortList::createPort(unsigned short, unsigned char)' may return constant 'NULL' at line 671, called at line 877. Null pointer introduced at line 671 in function 'PortList::createPort(unsigned short, unsigned char)'.
It was not actually possible for PortList::createPort to return NULL, but I made some code changes to make this more clear.
Error: Null pointer dereference (CWE 476) Read from null pointer 'ServiceNFO::currentProbe(this)' at line 1813 of components/nmap/build/amd64/service_scan.cc in function 'ServiceNFO::currentprobe_timemsleft(timeval const*)'. Function 'ServiceNFO::currentProbe()' may return constant 'NULL' at line 1707, called at line 1813. Null pointer introduced at line 1707 in function 'ServiceNFO::currentProbe()'.
I believe that at this point, there was already known to be a current probe, so that currentProbe would not return NULL. This wasn't very clear though, and I made some code changes to clarify it. David Fifield
Attachment:
nmap-6.25-parfait.patch
Description:
_______________________________________________ Sent through the dev mailing list http://nmap.org/mailman/listinfo/dev Archived at http://seclists.org/nmap-dev/
Current thread:
- Parfait analysis of nmap 6.25 Rich Burridge (Dec 10)
- Re: Parfait analysis of nmap 6.25 David Fifield (Dec 21)