View Issue Details

IDProjectCategoryView StatusLast Update
0000580LDMud 3.3Implementationpublic2018-01-29 21:57
Reporterzesstra Assigned Tozesstra  
PriorityhighSeveritycrashReproducibilityalways
Status resolvedResolutionfixed 
Fixed in Version3.3.718 
Summary0000580: Potential crash in load_object() due to stack overflow
DescriptionLarge strings as arguments to load_object() in simulate.c may lead to crashes.
Steps To Reproduceload_object("a"*8000000);
TagsNo tags attached.
Attached Files
load_object.diff (2,398 bytes)   
Index: simulate.c
===================================================================
--- simulate.c	(Revision 2426)
+++ simulate.c	(Arbeitskopie)
@@ -1792,11 +1792,16 @@
     /* We need two copies of <lname>: one to construct the filename in,
      * the second because lname might be a buffer which is deleted
      * during the compilation process.
+     * The memory is allocated in one chunk for both strings and an error 
+     * handler is pushed on the stack (the 2 is for '/' and '\0' in the object
+     * name, the 4 for '/', '\0' and the additional ".c" of the filename).
      */
-    name = alloca(name_length+2);
-    fname = alloca(name_length+4);
-    if (!name || !fname)
-        fatal("Stack overflow in load_object()");
+    name = xalloc_with_error_handler(2 * name_length + 2 + 4);
+    fname = name + name_length + 2 + 1;
+    if (!name)
+        errorf("Out of memory (%zu bytes) in load_object() for temporary name "
+               "buffers.\n", 2*name_length + 2 + 4);
+    
     if (!compat_mode)
         *name++ = '/';  /* Add and hide a leading '/' */
     strcpy(name, lname);
@@ -1825,6 +1830,7 @@
                  * memory... strange, but thinkable
                  */
                 errorf("Out of memory: unswap object '%s'\n", get_txt(ob->name));
+            pop_stack(); /* free error handler */
             return ob;
         }
     }
@@ -1892,6 +1898,7 @@
                         ob->flags &= ~O_CLONE;
                         ob->flags |= O_REPLACED;
                     }
+                    pop_stack(); /* free error handler */
                     return ob;
                 }
             }
@@ -1914,6 +1921,7 @@
                     ob->flags &= ~O_CLONE;
                     ob->flags |= O_REPLACED;
                 }
+                pop_stack(); /* free error handler */
                 return ob;
             }
             fname[name_length] = '.';
@@ -1945,6 +1953,7 @@
         ob = lookup_object_hash_str((char *)name);
         if (ob)
         {
+            pop_stack(); /* free error handler */
             return ob;
         }
 
@@ -2152,6 +2161,9 @@
     if ( !(ob->flags & O_DESTRUCTED))
         ob->flags |= O_WILL_CLEAN_UP;
 
+    /* free the error handler with the buffer for name and fname. */
+    pop_stack();
+    
     /* Restore the command giver */
     command_giver = check_object(save_command_giver);
 
load_object.diff (2,398 bytes)   

Relationships

child of 0000545 new Usages of alloca() have to be checked for possible stack overflow 

Activities

zesstra

2008-10-04 13:02

administrator   ~0000800

I attached a draft of a patch.

zesstra

2008-12-14 17:05

administrator   ~0000820

Patch applied in r2449.

Issue History

Date Modified Username Field Change
2008-10-01 15:06 zesstra New Issue
2008-10-01 15:06 zesstra Status new => assigned
2008-10-01 15:06 zesstra Assigned To => zesstra
2008-10-01 15:07 zesstra Relationship added child of 0000545
2008-10-04 13:02 zesstra File Added: load_object.diff
2008-10-04 13:02 zesstra Note Added: 0000800
2008-12-14 17:05 zesstra Status assigned => resolved
2008-12-14 17:05 zesstra Fixed in Version => 3.3.718
2008-12-14 17:05 zesstra Resolution open => fixed
2008-12-14 17:05 zesstra Note Added: 0000820
2010-11-16 09:42 zesstra Source_changeset_attached => ldmud.git master d47f7dc2
2018-01-29 18:59 zesstra Source_changeset_attached => ldmud.git master d47f7dc2
2018-01-29 21:57 zesstra Source_changeset_attached => ldmud.git master d47f7dc2