View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0000580 | LDMud 3.3 | Implementation | public | 2008-10-01 17:06 | 2018-01-29 22: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. | ||||
child of | 0000545 | new | Usages of alloca() have to be checked for possible stack overflow |
2008-10-04 15:02
|
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); |
|
I attached a draft of a patch. |
|
Patch applied in r2449. |
Date Modified | Username | Field | Change |
---|---|---|---|
2008-10-01 17:06 | zesstra | New Issue | |
2008-10-01 17:06 | zesstra | Status | new => assigned |
2008-10-01 17:06 | zesstra | Assigned To | => zesstra |
2008-10-01 17:07 | zesstra | Relationship added | child of 0000545 |
2008-10-04 15:02 | zesstra | File Added: load_object.diff | |
2008-10-04 15:02 | zesstra | Note Added: 0000800 | |
2008-12-14 18:05 | zesstra | Status | assigned => resolved |
2008-12-14 18:05 | zesstra | Fixed in Version | => 3.3.718 |
2008-12-14 18:05 | zesstra | Resolution | open => fixed |
2008-12-14 18:05 | zesstra | Note Added: 0000820 | |
2010-11-16 10:42 | zesstra | Source_changeset_attached | => ldmud.git master d47f7dc2 |
2018-01-29 19:59 | zesstra | Source_changeset_attached | => ldmud.git master d47f7dc2 |
2018-01-29 22:57 | zesstra | Source_changeset_attached | => ldmud.git master d47f7dc2 |