Fix use after free in core. #12138
Fix use after free in core. #12138
Conversation
Codecov Report
@@ Coverage Diff @@
## master #12138 +/- ##
==========================================
- Coverage 37.16% 37.15% -0.01%
==========================================
Files 902 902
Lines 288475 288475
==========================================
- Hits 107210 107185 -25
- Misses 181265 181290 +25
Continue to review full report at Codecov.
|
|
Cc @thestr4ng3r |
libr/core/task.c
Outdated
| @@ -182,6 +182,7 @@ static void task_free (RCoreTask *task) { | |||
| r_th_lock_free (task->dispatch_lock); | |||
| r_cons_context_free (task->cons_context); | |||
| free (task); | |||
| task = NULL; | |||
There was a problem hiding this comment.
What is the point of setting the parameter to NULL?
libr/core/core.c
Outdated
| @@ -2450,7 +2450,6 @@ R_API RCore *r_core_fini(RCore *c) { | |||
| r_list_free (c->files); | |||
| r_list_free (c->watchers); | |||
| r_list_free (c->scriptstack); | |||
| r_list_free (c->tasks); | |||
There was a problem hiding this comment.
The free function saved in core->tasks is r_core_task_decref, freeing the list should just decrease the refcount for each task as you would expect. If this leads to a uaf in some case, the problem is somewhere else, maybe somewhere the refcount is not increased where it should. If you have a case where this happens, please elaborate.
Also, you still have to free the RList itself or it will leak.
There was a problem hiding this comment.
you re right this was the wrong approach but there is genuinely an UAF case. With the other diff you can see why there is.
There was a problem hiding this comment.
Yes, you are absolutely right. Thanks for noticing, the fix is now correct.
The tasks are already freed by the reference counting.