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.

diff --git a/src/cadet/gnunet-service-cadet_peer.c b/src/cadet/gnunet-service-cadet_peer.c
index 71c7c67d0c11af201e23b08dd201720eb82719f0..c4e2c0ccf3f00be32a27de7b4027f8e8661edd19 100644 (file)
--- 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) ||