gnunet

commit 012ff13acc0cb2f5d7210aa48819395fecf12a3d
parent 2492acd4208bf14bf7747d235cfb9971f2cfbf99
Author: David Barksdale <amatus@amat.us>
Date:   Mon, 18 Dec 2017 18:39:03 -0600

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.

Diffstat:
M
src/cadet/gnunet-service-cadet_peer.c
 | 
51
++++++++++++++++++++++++++++++++++-----------------

1 file changed, 34 insertions(+), 17 deletions(-)
diff --git 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) ||