I was looking into how libotr and libgcrypt handle failing memory
allocations. I see that libotr generally tries to handle it by returning
an error from the function, but there are many libgcrypt calls that can
fail with "out of memory" that currently aren't checked, for example
gcry_mpi_print() and gcry_mpi_scan(). I'm not sure if they can cause any
bad problems, but silent failures aren't very good either.
I guess one solution would be to add more out of memory error checks,
but since it's so difficult to remember to check for them everywhere,
I'd rather be safe and just die immediately. libgcrypt has also several
memory allocations internally that die if they can't get the memory, so
you can't prevent dying entirely anyway.
Attached a patch to always die early from memory allocation failures in
libgcrypto. I was also considering changing all malloc()/free()s in
libotr to gcry_malloc()/gcry_free() so this would catch them too, but
that's too much work for now.
diff --git a/src/mem.c b/src/mem.c
index f01a8fa..1c8d4b9 100644
--- a/src/mem.c
+++ b/src/mem.c
@@ -35,9 +35,7 @@
/* #define OTRL_MEM_MAGIC 0x31415926 */
/* system headers */
-#ifdef OTRL_MEM_MAGIC
#include <stdio.h>
-#endif
#include <stdlib.h>
/* libgcrypt headers */
@@ -55,9 +53,13 @@ static void *otrl_mem_malloc(size_t n)
new_n += header_size;
/* Check for overflow attack */
- if (new_n < n) return NULL;
+ if (new_n < n) abort();
p = malloc(new_n);
- if (p == NULL) return NULL;
+ if (p == NULL) {
+ fprintf(stderr, "Out of memory when allocating %lu bytes\n",
+ (unsigned long)n);
+ exit(99);
+ }
((size_t *)p)[0] = new_n; /* Includes header size */
#ifdef OTRL_MEM_MAGIC
@@ -111,12 +113,12 @@ static void *otrl_mem_realloc(void *p, size_t n)
new_n += header_size;
/* Check for overflow attack */
- if (new_n < n) return NULL;
+ if (new_n < n) abort();
#ifdef OTRL_MEM_MAGIC
if (magic != OTRL_MEM_MAGIC) {
fprintf(stderr, "Illegal realloc!\n");
- return NULL;
+ abort();
}
#endif
@@ -133,7 +135,11 @@ static void *otrl_mem_realloc(void *p, size_t n)
new_p = real_p;
} else {
new_p = realloc(real_p, new_n);
- if (new_p == NULL) return NULL;
+ if (new_p == NULL) {
+ fprintf(stderr, "Out of memory when reallocating %lu bytes\n",
+ (unsigned long)new_n);
+ exit(99);
+ }
}
((size_t *)new_p)[0] = new_n; /* Includes header size */
_______________________________________________
OTR-dev mailing list
[email protected]
http://lists.cypherpunks.ca/mailman/listinfo/otr-dev
