From 012ff13acc0cb2f5d7210aa48819395fecf12a3d Mon Sep 17 00:00:00 2001
From: David Barksdale <amatus@amat.us>
Date: Mon, 18 Dec 2017 18:39:03 -0600
Subject: Fix use-after-free in loop over modified list

This loop saved the next pointer which is OK if the current element is
being freed, but the callback can cause other elements to be freed which
was detected by ASAN.
---
 src/cadet/gnunet-service-cadet_peer.c | 51 +++++++++++++++++++++++------------
 1 file changed, 34 insertions(+), 17 deletions(-)

(limited to 'src/cadet')

diff --git a/src/cadet/gnunet-service-cadet_peer.c b/src/cadet/gnunet-service-cadet_peer.c
index 71c7c67d0..c4e2c0ccf 100644
--- a/src/cadet/gnunet-service-cadet_peer.c
+++ b/src/cadet/gnunet-service-cadet_peer.c
@@ -532,32 +532,49 @@ GCP_set_mq (struct CadetPeer *cp,
        GCP_2s (cp),
        mq);
   cp->core_mq = mq;
-  for (struct GCP_MessageQueueManager *mqm = cp->mqm_head, *next;
+  /* Since these callbacks can remove any items from this list, we must take a
+   * snapshot and then test each one to see if it's still in the list. */
+  int count = 0;
+  for (struct GCP_MessageQueueManager *mqm = cp->mqm_head;
        NULL != mqm;
-       mqm = next)
+       mqm = mqm->next)
+    ++count;
+  struct GCP_MessageQueueManager *mqms[count];
+  int i = 0;
+  for (struct GCP_MessageQueueManager *mqm = cp->mqm_head;
+       NULL != mqm;
+       mqm = mqm->next)
+    mqms[i++] = mqm;
+  for (i = 0; i < count; ++i)
   {
-    /* Save next pointer in case mqm gets freed by the callback */
-    next = mqm->next;
-    if (NULL == mq)
+    for (struct GCP_MessageQueueManager *mqm = cp->mqm_head;
+         NULL != mqm;
+         mqm = mqm->next)
     {
-      if (NULL != mqm->env)
+      if (mqms[i] != mqm)
+        continue;
+      if (NULL == mq)
       {
-        GNUNET_MQ_discard (mqm->env);
-        mqm->env = NULL;
-        mqm->cb (mqm->cb_cls,
-                 GNUNET_SYSERR);
+        if (NULL != mqm->env)
+        {
+          GNUNET_MQ_discard (mqm->env);
+          mqm->env = NULL;
+          mqm->cb (mqm->cb_cls,
+                   GNUNET_SYSERR);
+        }
+        else
+        {
+          mqm->cb (mqm->cb_cls,
+                   GNUNET_NO);
+        }
       }
       else
       {
+        GNUNET_assert (NULL == mqm->env);
         mqm->cb (mqm->cb_cls,
-                 GNUNET_NO);
+                 GNUNET_YES);
       }
-    }
-    else
-    {
-      GNUNET_assert (NULL == mqm->env);
-      mqm->cb (mqm->cb_cls,
-               GNUNET_YES);
+      break;
     }
   }
   if ( (NULL != mq) ||
-- 
cgit v1.2.3