Crash in display_nmap_version() #1112

Closed
gvanem opened this Issue Jan 23, 2018 · 6 comments

Comments

Projects
None yet
2 participants

gvanem commented Jan 23, 2018

A simple nmap -V could cause a crash in display_nmap_version(). The reason seems to be this piece of pcap-version code:

#ifdef WIN32
  const char *pcap_num = strstr(pcap_version, "version ");
  if (pcap_num) {
    pcap_num += strlen("version ");
  }
  std::string pcap_num_str (pcap_num, strchr(pcap_num, ',') - pcap_num);

When there's no comma after pcap_num, it tries to push_back an almost infinite string!
When building with the internal Nmap libpcap, this does not happen. But when using the official libpcap, it does. Blame the horrid WIN32 code in pcap_lib_version().

(For me, it's no longer possible to build using MSVC-2017 and the internal libpcap).

gvanem commented Jan 23, 2018

Here is a patch that fixes the crash:

--- a/nmap.cc 2018-01-22 21:49:31
+++ b/nmap.cc 2018-01-23 09:34:32
@@ -2749,14 +2749,35 @@

   const char *pcap_version = pcap_lib_version();
 #ifdef WIN32
+  const char *comma = NULL;
   const char *pcap_num = strstr(pcap_version, "version ");
+  size_t str_len;
+
   if (pcap_num) {
     pcap_num += strlen("version ");
+    comma = strchr(pcap_num, ',');
+  }
+
+  if (comma)
+  {
+    /* E.g. pcap_num = "version 4.0, based on libpcap version 1.x.y"
+     */
+    str_len = strchr(pcap_num, ',') - pcap_num;
   }
-  std::string pcap_num_str (pcap_num, strchr(pcap_num, ',') - pcap_num);
+  else
+  {
+    /* E.g. pcap_num = "version libpcap version 1.9.0-PRE-GIT (packet.dll version 4.1.0.2980)"
+     */
+    while (!isdigit(*pcap_num))
+          pcap_num++;
+    str_len = strlen(pcap_num);
+  }
+
+  std::string pcap_num_str (pcap_num, str_len);
 #else
   std::string pcap_num_str = get_word_or_quote(pcap_version, 2);
 #endif

For me, using nmap -V, gives this:

Nmap version 7.60SVN ( http://nmap.org )
Platform: i686-pc-windows-windows (MSVC)
Compiled with: ... libpcap 1.9.0-PRE-GIT (packet.dll version 4.1.0.2980) ...
Available nsock engines: iocp poll select

I'm not sure if the (packet.dll ...) part is supposed to be there. What would a NPcap version print?

Could you please provide the entire string returned by pcap_lib_version() for your "1.9.0-PRE-GIT" instance?

gvanem commented Jan 31, 2018

It is:

libpcap version libpcap version 1.9.0-PRE-GIT (packet.dll version 4.1.0.2980)

Yes, there are 2 libpcap version in there. But no comma.

nnposter commented Jan 31, 2018

I am thinking of making the code slightly more robust:

  • The original code still has a hard dependency on "version " being present in pcap_version
  • Your fix has a dependency on digits being present in pcap_version

What if we do this instead?

--- a/nmap.cc
+++ b/nmap.cc
@@ -2746,11 +2746,10 @@
 
   const char *pcap_version = pcap_lib_version();
 #ifdef WIN32
-  const char *pcap_num = strstr(pcap_version, "version ");
-  if (pcap_num) {
-    pcap_num += strlen("version ");
-  }
-  std::string pcap_num_str (pcap_num, strchr(pcap_num, ',') - pcap_num);
+  const char *pcap_num = strpbrk(pcap_version, "0123456789");
+  if (pcap_num == NULL)
+    pcap_num = "(unknown)";
+  std::string pcap_num_str (pcap_num, strcspn(pcap_num, ","));
 #else
   std::string pcap_num_str = get_word_or_quote(pcap_version, 2);
 #endif

Tested cases:

pcap_version: WinPcap version 4.1.3 (packet.dll version 4.1.0.2980), based on libpcap version 1.0 branch 1_0_rel0b (20091008)
pcap_num_str: 4.1.3 (packet.dll version 4.1.0.2980)
output: WinPcap-4.1.3 (packet.dll version 4.1.0.2980)

pcap_version: Npcap version 0.98, based on libpcap version 1.8.1
pcap_num_str: 0.98
output: Npcap-0.98

pcap_version: libpcap version libpcap version 1.9.0-PRE-GIT (packet.dll version 4.1.0.2980)
pcap_num_str: 1.9.0-PRE-GIT (packet.dll version 4.1.0.2980)
output: libpcap-1.9.0-PRE-GIT (packet.dll version 4.1.0.2980)

pcap_version: libfoo bar baz
pcap_num_str: (unknown)
output: libfoo-(unknown)

pcap_version: libfoo
pcap_num_str: (unknown)
output: libfoo-(unknown)

pcap_version: (empty string)
pcap_num_str: (unknown)
output: ()-(unknown)

gvanem commented Feb 1, 2018

Much better. I've tested your patch with the official libpcap and it works fine.

nnposter commented Feb 1, 2018

Committed as r37128. Thank you for identifying the bug and the test strings.

@nmap-bot nmap-bot closed this in 6889a2f Feb 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment