cuckoosandbox / cuckoo

Cuckoo Sandbox is an automated dynamic malware analysis system
http://www.cuckoosandbox.org
Other
5.55k stars 1.71k forks source link

Add sanity checks for malloc/calloc in Cuckoo Sandbox Monitor #681

Closed dogbert2 closed 8 years ago

dogbert2 commented 8 years ago

Hello All,

In reviewing code in the current cuckoo-master file, I found some instances where calls to malloc() and calloc() are made without the appropriate checks for a return value of NULL, which indicates failure.

The patch files below add the necessary checks:

--- lookup.c.orig   2015-11-12 10:05:21.813240835 -0800
+++ lookup.c    2015-11-12 10:08:03.639808160 -0800
@@ -46,6 +46,10 @@
 void *lookup_add(lookup_t *d, unsigned int id, unsigned int size)
 {
     entry_t *t = (entry_t *) malloc(sizeof(entry_t) + size);
+    if (t == NULL) {   /* oops, unable to allocate memory  */
+   perror("Unable to allocate memory in function: lookup_add...exiting...\n");
+   exit(-1);
+    }
     ENTER();
     *t = (entry_t) {
         .next = d->root,

=======================================================================

--- utf8.c.orig 2015-11-12 10:09:27.072412015 -0800
+++ utf8.c  2015-11-12 10:12:37.310590309 -0800
@@ -17,6 +17,7 @@
 */

 #include <stdio.h>
+#include <stdlib.h>
 #include <windows.h>
 #include "utf8.h"

@@ -73,6 +74,10 @@

     int encoded_length = utf8_strlen_ascii(str, length);
     char * utf8string = (char *) malloc(encoded_length+4);
+    if (utf8string == NULL) {
+   perror("Unable to allocate memory in function: utf8_string...exiting...\n");
+   exit (-1);
+    }
     *((int *) utf8string) = encoded_length;
     int pos = 4;

@@ -88,6 +93,10 @@

     int encoded_length = utf8_strlen_unicode(str, length);
     char * utf8string = (char *) malloc(encoded_length+4);
+    if (utf8string == NULL) {
+   perror("Unable to allocate memory in function: utf8_wstring...exiting...\n");
+   exit (-1);
+    }
     *((int *) utf8string) = encoded_length;
     int pos = 4;

@@ -95,4 +104,4 @@
         pos += utf8_encode(*str++, (unsigned char *) &utf8string[pos]);
     }
     return utf8string;
-}
\ No newline at end of file
+}

=======================================================================

--- hooking.c.orig  2015-11-12 10:15:24.717866913 -0800
+++ hooking.c   2015-11-12 10:16:41.163582770 -0800
@@ -18,6 +18,7 @@

 #include <stdio.h>
 #include <stddef.h>
+#include <stdlib.h>
 #include <windows.h>
 #include "hooking.h"
 #include "distorm.h"
@@ -765,6 +766,10 @@
 {
     if(hook_info() == NULL) {
         hook_info_t *info = (hook_info_t *) calloc(1, sizeof(hook_info_t)+TLS_HOOK_INFO_RETADDR_SPACE);
+   if (info == NULL) {
+       perror("Unable to allocate memory in function: ensure_valid_hook_info...exiting...\n");
+       exit (-1);
+   }
         info->retaddr_esp = (unsigned int) info + sizeof(hook_info_t) + TLS_HOOK_INFO_RETADDR_SPACE;
         __writefsdword(TLS_HOOK_INFO, (unsigned int) info);
     }

Questions, Comments, Suggestions, Complaints? :)

Bill Parker (wp02855 at gmail dot com)

jbremer commented 8 years ago

Hi @dogbert2, I appreciate you taking a look at Cuckoo. However, and sorry for that, cuckoomon has been deprecated in favor of the new monitor (https://github.com/jbremer/monitor yes it should moved to our Cuckoo repository soon), so if you could please concentrate your thoughts on that repository instead of cuckoomon that'd be great. Thanks!

dogbert2 commented 8 years ago

I'm downloading the monitor-master.zip so I can give it the once over :)

Bill

On Thu, Nov 12, 2015 at 11:29 PM, Jurriaan Bremer notifications@github.com wrote:

Closed #681 https://github.com/cuckoobox/cuckoo/issues/681.

— Reply to this email directly or view it on GitHub https://github.com/cuckoobox/cuckoo/issues/681#event-463324347.