View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0000580 | LDMud 3.3 | Implementation | public | 2008-10-01 15:06 | 2018-01-29 21:57 |
| Reporter | zesstra | Assigned To | zesstra | ||
| Priority | high | Severity | crash | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Fixed in Version | 3.3.718 | ||||
| Summary | 0000580: Potential crash in load_object() due to stack overflow | ||||
| Description | Large strings as arguments to load_object() in simulate.c may lead to crashes. | ||||
| Steps To Reproduce | load_object("a"*8000000); | ||||
| Tags | No 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);
| ||||
| child of | 0000545 | new | Usages of alloca() have to be checked for possible stack overflow |
| 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 |