Skip to content

fix double-free by implicit interface or early termination#1305

Merged
LocutusOfBorg merged 1 commit into
Ettercap:masterfrom
koeppea:fix-1302
Apr 24, 2026
Merged

fix double-free by implicit interface or early termination#1305
LocutusOfBorg merged 1 commit into
Ettercap:masterfrom
koeppea:fix-1302

Conversation

@koeppea

@koeppea koeppea commented Apr 19, 2026

Copy link
Copy Markdown
Member

Fix #1302
This pull request fixes two double-free conditions.
First of all, during startup, Ettercap is filtering the list of useful interfaces returned by libpcap's pcap_findalldevs(). Its doing so by allocating memory to hold the pcap_if_t structures including all its contents:

      /* take over entry in filtered list */
      SAFE_CALLOC(cdev, 1, sizeof(pcap_if_t));
      memcpy(cdev, dev, sizeof(pcap_if_t));

This shortened single-linked list is then stored under EC_GBL_PCAP->ifs.

However, the trap here is, that the structure also holds pointers allocated during libpcap's pcap_findalldevs(), like name or description. The above code, copies them 1:1, including these struct-members' values.
Now, if no interface is specified -i, the memory address of the name member (pointer), allocated within libpcap, is copied to the global variable EC_GBL_OPTIONS->iface a.k.a. ec_gbls->options->iface:

   } else {
      iface = EC_GBL_OPTIONS->iface ? EC_GBL_OPTIONS->iface : (EC_GBL_OPTIONS->iface = capture_default_if());
char* capture_default_if(void)
{
   /*
    * as per deprecation message of pcap_lookupdev() the new way to determine
    * the default interface, is to use the first interface detected by the call
    * of pcap_findalldevs(). This is already determined in capture_getifs() and
    * stored in EC_GBL_PCAP->ifs
    */

   if (EC_GBL_PCAP && EC_GBL_PCAP->ifs)
      return EC_GBL_PCAP->ifs->name;

   return NULL;
}

Since the memory was allocated by libpcap, the call of pcap_freealldevs() frees this memory. Subsequently, EC_GBL_OPTIONS->iface is freed again, leading to the first double-free condition:

void ec_globals_free(void)
{
 
   capture_freeifs();
/* omitted */
   EC_GBL_FREE(ec_gbls->options->iface);

The second one is similar:

void capture_freeifs(void)
{
   pcap_if_t *dev, *ndev;

   /* first free filtered list entries */
   for (dev = EC_GBL_PCAP->ifs; dev != NULL; dev = ndev) {
      /* save the next entry in the list and free memory for the entry */
      ndev = dev->next;
      SAFE_FREE(dev->description);
      SAFE_FREE(dev);
   }

   /* Finally free the complete data structure using libpcap */
   if (EC_GBL_PCAP && EC_GBL_PCAP->allifs)
      pcap_freealldevs(EC_GBL_PCAP->allifs);
}

Only dev was allocated by Ettercap in any case. dev->description only if libpcap didn't provide any description.
But when the description member was set by libpcap, the pointer in the original pcap_if_t structure points to the memory independently. Even SAFE_FREE(dev->description); doesn't help, since its only setting the copied member pointer to NULL, the original still points to the memory which just got freed.

I checked the source code of pcap_freealldevs(). Since pcap_freealldevs() frees dev->description only if its not NULL, we don't need to free dev->description separately providing us with a elegant fix to this issue..

		/*
		 * Free the name string.
		 */
		free(curdev->name);

		/*
		 * Free the description string, if any.
		 */
		if (curdev->description != NULL)
			free(curdev->description);

		/*
		 * Free the interface.
		 */
		free(curdev);

@LocutusOfBorg LocutusOfBorg merged commit b78fd08 into Ettercap:master Apr 24, 2026
2 checks passed
@LocutusOfBorg LocutusOfBorg deleted the fix-1302 branch April 24, 2026 07:17
@LocutusOfBorg

Copy link
Copy Markdown
Contributor

I really trust you there :D

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

Successfully merging this pull request may close these issues.

double free

2 participants