As part of the update of nmap to version 6.25, a 3PSC form was created:

http://psarc.us.oracle.com/arc/PSARC/2012/376/nmap_6.25_3PSC_20121218.txt

To answer one of the questions, a Parfait code analysis of the nmap 6.25

code was run. The results were reported upstream:

http://seclists.org/nmap-dev/2012/q4/412

The nmap maintainers analyzed these results and responded:

http://seclists.org/nmap-dev/2012/q4/504

This is the patch they generated, with just 'nmap-6.25/' prepended to

the filenames on the "^--- " and "+++ " diff lines.

From a632df5fbeecf6271aadbd3bcc1c927977a7ba2c Mon Sep 17 00:00:00 2001

From: David Fifield <[email protected]>

Date: Thu, 20 Dec 2012 22:22:49 -0800

Subject: [PATCH 1/4] Add an ncat_assert macro.

This is an assert that will remain even if NDEBUG is defined.

---

 ncat/util.h |    7 +++++++

 1 file changed, 7 insertions(+)

diff --git ncat/util.h ncat/util.h

index bf9b42e..18de755 100644

--- nmap-6.25/ncat/util.h

+++ nmap-6.25/ncat/util.h

@@ -127,6 +127,13 @@ void logdebug(const char *fmt, ...)

      __attribute__ ((format (printf, 1, 2)));

 /* handle errors */

+

+#define ncat_assert(expr) \

+do { \

+        if (!(expr)) \

+                bye("assertion failed: %s", #expr); \

+} while (0)

+

 void die(char *);

 void bye(const char *, ...)

-- 

1.7.10.4

From cf5b5e2f5fdd4f7744e91c097d0395736e0744ab Mon Sep 17 00:00:00 2001

From: David Fifield <[email protected]>

Date: Thu, 20 Dec 2012 22:32:03 -0800

Subject: [PATCH 2/4] Assert that get_fdinfo doesn't return NULL.

Resolves these Parfait reports

(http://seclists.org/nmap-dev/2012/q4/412).

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'.

---

 ncat/ncat_core.c   |    1 +

 ncat/ncat_listen.c |    6 ++++--

 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git ncat/ncat_core.c ncat/ncat_core.c

index e6cb16c..42cf389 100644

--- nmap-6.25/ncat/ncat_core.c

+++ nmap-6.25/ncat/ncat_core.c

@@ -364,6 +364,7 @@ int ncat_broadcast(fd_set *fds, const fd_list_t *fdlist, const char *msg, size_t

             continue;

         fdn = get_fdinfo(fdlist, i);

+        ncat_assert(fdn != NULL);

         if (blocking_fdinfo_send(fdn, msg, size) <= 0) {

             if (o.debug > 1)

                 logdebug("Error sending to fd %d: %s.\n", i, socket_strerror(socket_errno()));

diff --git ncat/ncat_listen.c ncat/ncat_listen.c

index 5a0b502..3e0a104 100644

--- nmap-6.25/ncat/ncat_listen.c

+++ nmap-6.25/ncat/ncat_listen.c

@@ -293,6 +293,7 @@ static int ncat_listen_stream(int proto)

                 FD_CLR(i, &master_readfds);

                 FD_CLR(i, &master_writefds);

                 fdi = get_fdinfo(&client_fdlist, i);

+                ncat_assert(fdi != NULL);

                 switch (ssl_handshake(fdi)) {

                 case NCAT_SSL_HANDSHAKE_COMPLETED:

                     /* Clear from sslpending_fds once ssl is established */

@@ -535,7 +536,7 @@ int read_socket(int recv_fd)

     int nbytes, pending;

     fdn = get_fdinfo(&client_fdlist, recv_fd);

-    assert(fdn != NULL);

+    ncat_assert(fdn != NULL);

     nbytes = 0;

     do {

@@ -838,7 +839,7 @@ static void read_and_broadcast(int recv_fd)

     int pending;

     fdn = get_fdinfo(&client_fdlist, recv_fd);

-    assert(fdn);

+    ncat_assert(fdn != NULL);

     /* Loop while ncat_recv indicates data is pending. */

     do {

@@ -943,6 +944,7 @@ static void shutdown_sockets(int how)

             continue;

         fdn = get_fdinfo(&broadcast_fdlist, i);

+        ncat_assert(fdn != NULL);

         shutdown(fdn->fd, how);

     }

 }

-- 

1.7.10.4

From b7b822c8a0f4810d19d5b061c6fbaac0a2ad5723 Mon Sep 17 00:00:00 2001

From: David Fifield <[email protected]>

Date: Fri, 21 Dec 2012 12:22:30 -0800

Subject: [PATCH 3/4] Make PortList::mapPort return void.

Contrary to the doc comment, this function could never return false but

only cause a fatal error.

Resolves these Parfait reports

(http://seclists.org/nmap-dev/2012/q4/412).

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)'.

---

 portlist.cc |   17 +++++------------

 portlist.h  |    2 +-

 2 files changed, 6 insertions(+), 13 deletions(-)

diff --git portlist.cc portlist.cc

index 0c8af80..cd40d04 100644

--- nmap-6.25/portlist.cc

+++ nmap-6.25/portlist.cc

@@ -629,8 +629,8 @@ Port *PortList::nextPort(const Port *cur, Port *next,

 }

 /* Convert portno and protocol into the internal indices used to index

-   port_list. Returns false on error, true otherwise. */

-bool PortList::mapPort(u16 *portno, u8 *protocol) const {

+   port_list. */

+void PortList::mapPort(u16 *portno, u8 *protocol) const {

   int mapped_portno, mapped_protocol;

   mapped_protocol = INPROTO2PORTLISTPROTO(*protocol);

@@ -638,7 +638,6 @@ bool PortList::mapPort(u16 *portno, u8 *protocol) const {

   if (*protocol == IPPROTO_IP)

     assert(*portno < 256);

   if(port_map[mapped_protocol]==NULL || port_list[mapped_protocol]==NULL) {

-    assert(0);

     fatal("%s(%i,%i): you're trying to access uninitialized protocol", __func__, *portno, *protocol);

   }

   mapped_portno = port_map[mapped_protocol][*portno];

@@ -648,14 +647,10 @@ bool PortList::mapPort(u16 *portno, u8 *protocol) const {

   *portno = mapped_portno;

   *protocol = mapped_protocol;

-

-  return true;

 }

 const Port *PortList::lookupPort(u16 portno, u8 protocol) const {

-  if (!mapPort(&portno, &protocol))

-    return NULL;

-

+  mapPort(&portno, &protocol);

   return port_list[protocol][portno];

 }

@@ -667,8 +662,7 @@ Port *PortList::createPort(u16 portno, u8 protocol) {

   mapped_portno = portno;

   mapped_protocol = protocol;

-  if (!mapPort(&mapped_portno, &mapped_protocol))

-    return NULL;

+  mapPort(&mapped_portno, &mapped_protocol);

   p = port_list[mapped_protocol][mapped_portno];

   if (p == NULL) {

@@ -688,8 +682,7 @@ int PortList::forgetPort(u16 portno, u8 protocol) {

   log_write(LOG_PLAIN, "Removed %d\n", portno);

-  if (!mapPort(&portno, &protocol))

-    return -1;

+  mapPort(&portno, &protocol);

   answer = port_list[protocol][portno];

   if (answer == NULL)

diff --git portlist.h portlist.h

index 8eaee1c..d7faf71 100644

--- nmap-6.25/portlist.h

+++ nmap-6.25/portlist.h

@@ -290,7 +290,7 @@ class PortList {

   bool hasOpenPorts() const;

  private:

-  bool mapPort(u16 *portno, u8 *protocol) const;

+  void mapPort(u16 *portno, u8 *protocol) const;

   /* Get Port structure from PortList structure.*/

   const Port *lookupPort(u16 portno, u8 protocol) const;

   Port *createPort(u16 portno, u8 protocol);

-- 

1.7.10.4

From 536eb67f24f9f101cd4aa1c82510ebbe8086923d Mon Sep 17 00:00:00 2001

From: David Fifield <[email protected]>

Date: Fri, 21 Dec 2012 13:09:56 -0800

Subject: [PATCH 4/4] Make ServiceNFO::currentprobe_timemsleft take a probe

 argument.

It seems that this function was usually called after having called

currentProbe outside the call to currentprobe_timemsleft, with the call

to currentProbe inside the function having the same result. This is a

bit tenuous, so make the probe we're talking about explicit.

Resolves these Parfait reports

(http://seclists.org/nmap-dev/2012/q4/412).

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()'.

---

 service_scan.cc |   25 ++++++++++++++-----------

 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git service_scan.cc service_scan.cc

index b800665..9428a9a 100644

--- nmap-6.25/service_scan.cc

+++ nmap-6.25/service_scan.cc

@@ -191,7 +191,7 @@ public:

   // Number of milliseconds left to complete the present probe, or 0 if

   // the probe is already expired.  Timeval can omitted, it is just there 

   // as an optimization in case you have it handy.

-  int currentprobe_timemsleft(const struct timeval *now = NULL);

+  int probe_timemsleft(const ServiceProbe *probe, const struct timeval *now = NULL);

   enum serviceprobestate probe_state; // defined in portlist.h

   nsock_iod niod; // The IO Descriptor being used in this probe (or NULL)

   u16 portno; // in host byte order

@@ -1799,7 +1799,7 @@ void ServiceNFO::resetProbes(bool freefp) {

 }

-int ServiceNFO::currentprobe_timemsleft(const struct timeval *now) {

+int ServiceNFO::probe_timemsleft(const ServiceProbe *probe, const struct timeval *now) {

   int timeused, timeleft;

   if (now)

@@ -1810,7 +1810,11 @@ int ServiceNFO::currentprobe_timemsleft(const struct timeval *now) {

     timeused = TIMEVAL_MSEC_SUBTRACT(tv, currentprobe_exec_time);

   }

-  timeleft = currentProbe()->totalwaitms - timeused;

+  // Historically this function was always called with the assumption that

+  // probe == currentProbe(). Check that this remains the case.

+  assert(probe == currentProbe());

+

+  timeleft = probe->totalwaitms - timeused;

   return (timeleft < 0)? 0 : timeleft;

 }

@@ -1941,7 +1945,7 @@ static void adjustPortStateIfNecessary(ServiceNFO *svc) {

     probestring = probe->getProbeString(&probestringlen);

     assert(probestringlen > 0);

     // Now we write the string to the IOD

-    nsock_write(nsp, nsi, servicescan_write_handler, svc->currentprobe_timemsleft(), svc,

+    nsock_write(nsp, nsi, servicescan_write_handler, svc->probe_timemsleft(probe), svc,

 		(const char *) probestring, probestringlen);

     return 0;

   }

@@ -1967,7 +1971,7 @@ static void startNextProbe(nsock_pool nsp, nsock_iod nsi, ServiceGroup *SG,

       svc->currentprobe_exec_time = *nsock_gettimeofday();

       send_probe_text(nsp, nsi, svc, probe);

       nsock_read(nsp, nsi, servicescan_read_handler, 

-		 svc->currentprobe_timemsleft(nsock_gettimeofday()), svc);

+		 svc->probe_timemsleft(probe, nsock_gettimeofday()), svc);

     } else {

       // Should only happen if someone has a highly perverse nmap-service-probes

       // file.  Null scan should generally never be the only probe.

@@ -2016,7 +2020,7 @@ static void startNextProbe(nsock_pool nsp, nsock_iod nsi, ServiceGroup *SG,

 	send_probe_text(nsp, nsi, svc, probe);

 	// Now let us read any results

 	nsock_read(nsp, nsi, servicescan_read_handler, 

-		   svc->currentprobe_timemsleft(nsock_gettimeofday()), svc);

+		   svc->probe_timemsleft(probe, nsock_gettimeofday()), svc);

       }

     } else {

       // No more probes remaining!  Failed to match

@@ -2261,7 +2265,7 @@ static void servicescan_connect_handler(nsock_pool nsp, nsock_event nse, void *m

     svc->currentprobe_exec_time = *nsock_gettimeofday();

     send_probe_text(nsp, nsi, svc, probe);

     // Now let us read any results

-    nsock_read(nsp, nsi, servicescan_read_handler, svc->currentprobe_timemsleft(nsock_gettimeofday()), svc);

+    nsock_read(nsp, nsi, servicescan_read_handler, svc->probe_timemsleft(probe, nsock_gettimeofday()), svc);

   } else if (status == NSE_STATUS_TIMEOUT || status == NSE_STATUS_ERROR) {

       // This is not good.  The connect() really shouldn't generally

       // be timing out like that.  We'll mark this svc as incomplete

@@ -2421,13 +2425,12 @@ static void servicescan_read_handler(nsock_pool nsp, nsock_event nse, void *myda

       // to timeout.  For now I'll limit it to 4096 bytes just to

       // avoid reading megs from services like chargen.  But better

       // approach is needed.

-      if (svc->currentprobe_timemsleft() > 0 && readstrlen < 4096) { 

-	nsock_read(nsp, nsi, servicescan_read_handler, svc->currentprobe_timemsleft(), svc);

+      if (svc->probe_timemsleft(probe) > 0 && readstrlen < 4096) { 

+	nsock_read(nsp, nsi, servicescan_read_handler, svc->probe_timemsleft(probe), svc);

       } else {

 	// Failed -- lets go to the next probe.

 	if (readstrlen > 0)

-	  svc->addToServiceFingerprint(svc->currentProbe()->getName(), readstr, 

-				       readstrlen);

+	  svc->addToServiceFingerprint(probe->getName(), readstr, readstrlen);

 	startNextProbe(nsp, nsi, SG, svc, false);

       }

     }

-- 

1.7.10.4