Fix UAF when deleting hook while in hook callback #1127 (#1130)

* Handle the cpu context save in a more pythonic way, so the context can be serialized and reuse in an other process using the same emulator architecture and modes

* Fix type error ; mistakes a size_t uint64_t ; breaks in 32bit...

* Fix the UAF situation when deleting a hook while being in a hook callback. Added an attribute 'to_delete' to hooks, and a list hooks_to_del to delay the free of the hooks

* Minor fixes ; forgot return type of clear_deleted_hooks ; do not declare variable in for predicate

uc.c

@@ -530,6 +530,28 @@ static void hook_count_cb(struct uc_struct *uc, uint64_t address, uint32_t size,
         uc_emu_stop(uc);
 }
 
+static void clear_deleted_hooks(uc_engine *uc)
+{
+    struct list_item * cur;
+    struct hook * hook;
+    int i;
+    
+    for (cur = uc->hooks_to_del.head; cur != NULL && (hook = (struct hook *)cur->data); cur = cur->next)
+    {
+        assert(hook->to_delete);
+        for (i = 0; i < UC_HOOK_MAX; i++) {
+            if (list_remove(&uc->hook[i], (void *)hook)) {
+                if (--hook->refs == 0) {
+                    free(hook);
+                }
+                // a hook cannot be twice in the same list
+                break;
+            }
+        }
+    }
+    list_clear(&uc->hooks_to_del);
+}
+
 UNICORN_EXPORT
 uc_err uc_emu_start(uc_engine* uc, uint64_t begin, uint64_t until, uint64_t timeout, size_t count)
 {
@@ -631,6 +653,12 @@ uc_err uc_emu_start(uc_engine* uc, uint64_t begin, uint64_t until, uint64_t time
     // emulation is done
     uc->emulation_done = true;
 
+
+
+    // remove hooks to delete
+    clear_deleted_hooks(uc);
+
+
     if (timeout) {
         // wait for the timer to finish
         qemu_thread_join(&uc->timer);
@@ -1084,6 +1112,7 @@ uc_err uc_hook_add(uc_engine *uc, uc_hook *hh, int type, void *callback,
     hook->callback = callback;
     hook->user_data = user_data;
     hook->refs = 0;
+    hook->to_delete = false;
     *hh = (uc_hook)hook;
 
     // UC_HOOK_INSN has an extra argument for instruction ID
@@ -1151,24 +1180,26 @@ uc_err uc_hook_add(uc_engine *uc, uc_hook *hh, int type, void *callback,
     return ret;
 }
 
+
 UNICORN_EXPORT
 uc_err uc_hook_del(uc_engine *uc, uc_hook hh)
 {
     int i;
     struct hook *hook = (struct hook *)hh;
+
     // we can't dereference hook->type if hook is invalid
     // so for now we need to iterate over all possible types to remove the hook
     // which is less efficient
     // an optimization would be to align the hook pointer
     // and store the type mask in the hook pointer.
     for (i = 0; i < UC_HOOK_MAX; i++) {
-        if (list_remove(&uc->hook[i], (void *)hook)) {
-            if (--hook->refs == 0) {
-                free(hook);
-                break;
-            }
+        if (list_exists(&uc->hook[i], (void *) hook))
+        {
+            hook->to_delete = true;
+            list_append(&uc->hooks_to_del, hook);
         }
     }
+
     return UC_ERR_OK;
 }
 
@@ -1177,7 +1208,7 @@ void helper_uc_tracecode(int32_t size, uc_hook_type type, void *handle, int64_t
 void helper_uc_tracecode(int32_t size, uc_hook_type type, void *handle, int64_t address)
 {
     struct uc_struct *uc = handle;
-    struct list_item *cur = uc->hook[type].head;
+    struct list_item *cur;
     struct hook *hook;
 
     // sync PC in CPUArchState with address
@@ -1185,12 +1216,12 @@ void helper_uc_tracecode(int32_t size, uc_hook_type type, void *handle, int64_t
         uc->set_pc(uc, address);
     }
 
-    while (cur != NULL && !uc->stop_request) {
-        hook = (struct hook *)cur->data;
+    for (cur = uc->hook[type].head; cur != NULL && (hook = (struct hook *)cur->data); cur = cur->next){
+        if (hook->to_delete)
+            continue;
         if (HOOK_BOUND_CHECK(hook, (uint64_t)address)) {
             ((uc_cb_hookcode_t)hook->callback)(uc, address, size, hook->user_data);
         }
-        cur = cur->next;
     }
 }