Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash in display_nmap_version() #1112

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

Crash in display_nmap_version() #1112

gvanem opened this issue Jan 23, 2018 · 6 comments

Comments

@gvanem
Copy link

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
Copy link
Author

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?

@nnposter
Copy link

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

@gvanem
Copy link
Author

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
Copy link

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
Copy link
Author

gvanem commented Feb 1, 2018

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

@nnposter
Copy link

nnposter commented Feb 1, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants