Merge pull request #430 from ArnoStiefvater/pcap

resolve double free
greenbone · Nov 22, 2019 · 8ab6899 · 8ab6899
2 parents deb67ea + ae59ba3
commit 8ab6899
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 26 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,7 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - The logging of the NASL internal regexp functions was extended to include the pattern in case of a failed regcomp(). [#397](https://github.com/greenbone/openvas/pull/397)
 - Add config for gpg keyring path (OPENVAS_GPG_BASE_DIR) [#407](https://github.com/greenbone/openvas/pull/407)
 - Use __func__ instead of __FUNCTION__ [#419](https://github.com/greenbone/openvas/pull/419)
-- Use pcap_findalldevs() instead of deprecated function pcap_lookupdev() [#422](https://github.com/greenbone/openvas/pull/422)
+- Use pcap_findalldevs() instead of deprecated function pcap_lookupdev() [#422](https://github.com/greenbone/openvas/pull/422) [#430](https://github.com/greenbone/openvas/pull/430)
 
 [Unreleased]: https://github.com/greenbone/openvas/compare/openvas-7.0...master
 

diff --git a/misc/bpf_share.c b/misc/bpf_share.c
@@ -45,7 +45,6 @@ print_pcap_error (pcap_t *p, char *prefix)
 }
 
 /**
- * @param iface Name of interface. String has to be freed by caller.
  * @return -1 in case of error, index of the opened pcap_t in pcaps
  *         otherwise.
  */
@@ -70,12 +69,10 @@ bpf_open_live (char *iface, char *filter)
 
   if (iface == NULL)
     {
-      if (pcap_findalldevs (&alldevsp, errbuf) == -1)
+      if (pcap_findalldevs (&alldevsp, errbuf) < 0)
         g_message ("Error for pcap_findalldevs(): %s", errbuf);
       if (alldevsp != NULL)
-        /* get first device in list */
-        iface = g_strdup (alldevsp->name);
-      pcap_freealldevs (alldevsp);
+        iface = alldevsp->name;
     }
 
   ret = pcap_open_live (iface, 1500, 0, 1, errbuf);
@@ -114,6 +111,9 @@ bpf_open_live (char *iface, char *filter)
     }
   pcaps[i] = ret;
   pcap_freecode (&filter_prog);
+  if (alldevsp != NULL)
+    pcap_freealldevs (alldevsp);
+
   return i;
 }
 

diff --git a/nasl/capture_packet.c b/nasl/capture_packet.c
@@ -79,19 +79,18 @@ init_capture_device (struct in_addr src, struct in_addr dest, char *filter)
     }
   else
     {
-      if (pcap_findalldevs (&alldevsp, errbuf) == -1)
+      if (pcap_findalldevs (&alldevsp, errbuf) < 0)
         g_message ("Error for pcap_findalldevs(): %s", errbuf);
       if (alldevsp != NULL)
-        /* get first device in list */
-        interface = g_strdup (alldevsp->name);
+        interface = alldevsp->name;
       ret = bpf_open_live (interface, filter);
-      pcap_freealldevs (alldevsp);
     }
 
   if (free_filter != 0)
     g_free (filter);
 
-  g_free (interface);
+  if (alldevsp != NULL)
+    pcap_freealldevs (alldevsp);
 
   return ret;
 }
@@ -193,19 +192,18 @@ init_v6_capture_device (struct in6_addr src, struct in6_addr dest, char *filter)
     }
   else
     {
-      if (pcap_findalldevs (&alldevsp, errbuf) == -1)
+      if (pcap_findalldevs (&alldevsp, errbuf) < 0)
         g_message ("Error for pcap_findalldevs(): %s", errbuf);
       if (alldevsp != NULL)
-        /* get first device in list */
-        interface = g_strdup (alldevsp->name);
+        interface = alldevsp->name;
       ret = bpf_open_live (interface, filter);
-      pcap_freealldevs (alldevsp);
     }
 
   if (free_filter != 0)
     g_free (filter);
 
-  g_free (interface);
+  if (alldevsp != NULL)
+    pcap_freealldevs (alldevsp);
 
   return ret;
 }

diff --git a/nasl/nasl_packet_forgery.c b/nasl/nasl_packet_forgery.c
@@ -1538,20 +1538,17 @@ nasl_pcap_next (lex_ctxt *lexic)
         }
       if (interface == NULL)
         {
-          if (pcap_findalldevs (&alldevsp, errbuf) == -1)
+          if (pcap_findalldevs (&alldevsp, errbuf) < 0)
             g_message ("Error for pcap_findalldevs(): %s", errbuf);
           if (alldevsp != NULL)
-            /* get first device in list */
-            interface = g_strdup (alldevsp->name);
-          pcap_freealldevs (alldevsp);
+            interface = alldevsp->name;
         }
     }
 
   if (interface != NULL)
     {
       bpf = bpf_open_live (interface, filter);
     }
-  g_free (interface);
 
   if (bpf < 0)
     {
@@ -1637,6 +1634,9 @@ nasl_pcap_next (lex_ctxt *lexic)
     retc->x.str_val = (char *) ret6;
   retc->size = sz;
 
+  if (alldevsp != NULL)
+    pcap_freealldevs (alldevsp);
+
   return retc;
 }
 
@@ -1678,18 +1678,15 @@ nasl_send_capture (lex_ctxt *lexic)
         }
       if (interface == NULL)
         {
-          if (pcap_findalldevs (&alldevsp, errbuf) == -1)
+          if (pcap_findalldevs (&alldevsp, errbuf) < 0)
             g_message ("Error for pcap_findalldevs(): %s", errbuf);
           if (alldevsp != NULL)
-            /* get first device in list */
-            interface = g_strdup (alldevsp->name);
-          pcap_freealldevs (alldevsp);
+            interface = alldevsp->name;
         }
     }
 
   if (interface != NULL)
     bpf = bpf_open_live (interface, filter);
-  g_free (interface);
 
   if (bpf < 0)
     {
@@ -1774,5 +1771,8 @@ nasl_send_capture (lex_ctxt *lexic)
     retc->x.str_val = (char *) ret6;
   retc->size = sz;
 
+  if (alldevsp != NULL)
+    pcap_freealldevs (alldevsp);
+
   return retc;
 }