From 1710fc1a89578dfaaff684a1aafbc4d16e346f79 Mon Sep 17 00:00:00 2001
From: goldsimon <goldsimon@gmx.de>
Date: Mon, 25 Sep 2017 22:29:02 +0200
Subject: [PATCH] Fix a corner case of double-free in the heap

---
 src/core/mem.c            | 33 +++++++++++++++++++--
 test/unit/core/test_mem.c | 62 +++++++++++++++++++++++++++++++++++----
 2 files changed, 88 insertions(+), 7 deletions(-)

diff --git a/src/core/mem.c b/src/core/mem.c
index 14bdd904..689d7f0e 100644
--- a/src/core/mem.c
+++ b/src/core/mem.c
@@ -418,6 +418,25 @@ mem_init(void)
   }
 }
 
+/* Check if a struct mem is correctly linked.
+ * If not, double-free is a possible reason.
+ */
+static int
+mem_link_valid(struct mem *mem)
+{
+  struct mem *nmem, *pmem;
+  mem_size_t rmem_idx;
+  rmem_idx = (mem_size_t)((u8_t *)mem - ram);
+  nmem = (struct mem *)(void *)&ram[mem->next];
+  pmem = (struct mem *)(void *)&ram[mem->prev];
+  if ((mem->next > MEM_SIZE_ALIGNED) || (mem->prev > MEM_SIZE_ALIGNED) ||
+      ((mem->prev != rmem_idx) && (pmem->next != rmem_idx)) ||
+      ((nmem != ram_end) && (nmem->prev != rmem_idx))) {
+    return 0;
+  }
+  return 1;
+}
+
 /**
  * Put a struct mem back on the heap
  *
@@ -455,7 +474,7 @@ mem_free(void *rmem)
   }
   /* protect the heap from concurrent access */
   LWIP_MEM_FREE_PROTECT();
-  /* mem has to be in a used state ... */
+  /* mem has to be in a used state */
   if (!mem->used) {
     LWIP_MEM_ILLEGAL_FREE("mem_free: illegal memory: double free");
     LWIP_MEM_FREE_UNPROTECT();
@@ -464,7 +483,17 @@ mem_free(void *rmem)
     MEM_STATS_INC_LOCKED(illegal);
     return;
   }
-  /* ... and is now unused. */
+
+  if (!mem_link_valid(mem)) {
+    LWIP_MEM_ILLEGAL_FREE("mem_free: illegal memory: non-linked: double free");
+    LWIP_MEM_FREE_UNPROTECT();
+    LWIP_DEBUGF(MEM_DEBUG | LWIP_DBG_LEVEL_SEVERE, ("mem_free: illegal memory: non-linked: double free?\n"));
+    /* protect mem stats from concurrent access */
+    MEM_STATS_INC_LOCKED(illegal);
+    return;
+  }
+
+  /* mem is now unused. */
   mem->used = 0;
 
   if (mem < lfree) {
diff --git a/test/unit/core/test_mem.c b/test/unit/core/test_mem.c
index 8c6f7d0b..2aeb20cf 100644
--- a/test/unit/core/test_mem.c
+++ b/test/unit/core/test_mem.c
@@ -121,6 +121,7 @@ START_TEST(test_mem_invalid_free)
 
   ptr = (u8_t *)mem_malloc(1);
   fail_unless(ptr != NULL);
+  fail_unless(lwip_stats.mem.used != 0);
 
   ptr_low = ptr - 0x10;
   mem_free(ptr_low);
@@ -140,21 +141,72 @@ END_TEST
 
 START_TEST(test_mem_double_free)
 {
-  u8_t *ptr;
+  u8_t *ptr1b, *ptr1, *ptr2, *ptr3;
   LWIP_UNUSED_ARG(_i);
 
   fail_unless(lwip_stats.mem.used == 0);
+  fail_unless(lwip_stats.mem.illegal == 0);
 
-  ptr = (u8_t *)mem_malloc(1);
-  fail_unless(ptr != NULL);
+  ptr1 = (u8_t *)mem_malloc(1);
+  fail_unless(ptr1 != NULL);
+  fail_unless(lwip_stats.mem.used != 0);
 
-  mem_free(ptr);
+  ptr2 = (u8_t *)mem_malloc(1);
+  fail_unless(ptr2 != NULL);
+  fail_unless(lwip_stats.mem.used != 0);
+
+  ptr3 = (u8_t *)mem_malloc(1);
+  fail_unless(ptr3 != NULL);
+  fail_unless(lwip_stats.mem.used != 0);
+
+  /* free the middle mem */
+  mem_free(ptr2);
+  fail_unless(lwip_stats.mem.illegal == 0);
+
+  /* double-free of middle mem: should fail */
+  mem_free(ptr2);
+  fail_unless(lwip_stats.mem.illegal == 1);
+  lwip_stats.mem.illegal = 0;
+
+  /* free upper memory and try again */
+  mem_free(ptr3);
+  fail_unless(lwip_stats.mem.illegal == 0);
+
+  mem_free(ptr2);
+  fail_unless(lwip_stats.mem.illegal == 1);
+  lwip_stats.mem.illegal = 0;
+
+  /* free lower memory and try again */
+  mem_free(ptr1);
   fail_unless(lwip_stats.mem.illegal == 0);
   fail_unless(lwip_stats.mem.used == 0);
 
-  mem_free(ptr);
+  mem_free(ptr2);
   fail_unless(lwip_stats.mem.illegal == 1);
   fail_unless(lwip_stats.mem.used == 0);
+  lwip_stats.mem.illegal = 0;
+
+  /* reallocate lowest memory, now overlapping already freed ptr2 */
+#ifndef MIN_SIZE
+#define MIN_SIZE 12
+#endif
+  ptr1b = (u8_t *)mem_malloc(MIN_SIZE * 2);
+  fail_unless(ptr1b != NULL);
+  fail_unless(lwip_stats.mem.used != 0);
+
+  mem_free(ptr2);
+  fail_unless(lwip_stats.mem.illegal == 1);
+  lwip_stats.mem.illegal = 0;
+
+  memset(ptr1b, 1, MIN_SIZE * 2);
+
+  mem_free(ptr2);
+  fail_unless(lwip_stats.mem.illegal == 1);
+  lwip_stats.mem.illegal = 0;
+
+  mem_free(ptr1b);
+  fail_unless(lwip_stats.mem.illegal == 0);
+  fail_unless(lwip_stats.mem.used == 0);
 }
 END_TEST