about summary refs log tree commit diff
diff options
context:
space:
mode:
authorea <ea@users.noreply.github.com>2024-10-02 13:08:24 -0500
committerGitHub <noreply@github.com>2024-10-02 13:08:24 -0500
commit994ac558787cc10512854b85115e7f5e5e268513 (patch)
treef71d383c3b096858bd91a793e99f9dc6c5627be1
parentd21fb1a558b25c4f46692fa999c0028dfe0eecc0 (diff)
downloadafl++-994ac558787cc10512854b85115e7f5e5e268513.tar.gz
Fix uninitialized alloc_canary in libdislocator
When random alloc_canary env var option was introduced, a possibility for use of uninitialized alloc_canary value was made. 

In most cases, constructor will be called during shared library load and the alloc_canary would be initialized to either its default value or a randomly generated one if forced by AFL_RANDOM_ALLOC_CANARY env var.

However, in some cases, libraries loaded before libdislocator will make allocations (still using libdislocator's allocation functions) while alloc_canary is still uninitialized. In such cases, canary value is usually NULL. 
If such allocated value is then free()'d after libdislocator's constructor has been run, call to free() will fail causing a false positive. This condition usually happens while calling library destructors at process termination. 

The patch ensures the canary value is initialized in all cases, and introduces a destructor that reverts it to default value. 

This  does mean that certain number of early allocations will use the default canary value rather than the random one set afterwards.  This seems like a reasonable tradeoff as I haven't found a surefire way of forcing libdislocator's constructor to run first in all possible cases (if nothing else, libphtread usually  has priority).
-rw-r--r--utils/libdislocator/libdislocator.so.c9
1 files changed, 8 insertions, 1 deletions
diff --git a/utils/libdislocator/libdislocator.so.c b/utils/libdislocator/libdislocator.so.c
index b80be1a1..f41491b1 100644
--- a/utils/libdislocator/libdislocator.so.c
+++ b/utils/libdislocator/libdislocator.so.c
@@ -162,7 +162,7 @@ static u8     alloc_verbose,            /* Additional debug messages        */
 static _Atomic size_t total_mem;        /* Currently allocated mem          */
 
 static __thread u32 call_depth;         /* To avoid recursion via fprintf() */
-static u32          alloc_canary;
+static u32          alloc_canary = ALLOC_CANARY;
 
 /* This is the main alloc function. It allocates one page more than necessary,
    sets that tailing page to PROT_NONE, and then increments the return address
@@ -578,6 +578,13 @@ __attribute__((constructor)) void __dislocator_init(void) {
 
 }
 
+__attribute__((destructor)) void __dislocator_fini(void) {
+
+  alloc_canary = ALLOC_CANARY; // restore to default canary value
+
+}
+
+   
 /* NetBSD fault handler specific api subset */
 
 void (*esetfunc(void (*fn)(int, const char *, ...)))(int, const char *, ...) {