View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0000581 | LDMud 3.3 | Implementation | public | 2008-10-01 15:13 | 2018-01-29 21:57 |
Reporter | zesstra | Assigned To | zesstra | ||
Priority | high | Severity | crash | Reproducibility | always |
Status | resolved | Resolution | fixed | ||
Product Version | 3.3 | ||||
Fixed in Version | 3.3.718 | ||||
Summary | 0000581: Potential crashes in rename_object() and replace_program() due to stack overflows | ||||
Description | rename_object() and replace_program() in object.c may crash with large argument strings. Note: the crash in rename_object() occurs _before_ the call to privilege_violation. Therefore any non-privileged object can cause a crash. | ||||
Steps To Reproduce | replace_program("a"*8400000); rename_object(ME, "a"*8500000+".c"); | ||||
Tags | No tags attached. | ||||
Attached Files | objects.diff (5,330 bytes)
Index: object.c =================================================================== --- object.c (Revision 2435) +++ object.c (Arbeitskopie) @@ -3282,10 +3282,11 @@ */ { - object_t *ob; - char *name; - string_t *m_name; - mp_int length; + object_t *ob; /* object to be renamed */ + char *name; /* new name as c-string */ + string_t *m_name; /* new name */ + size_t length; /* length of new name */ + Bool freenamebuffer = MY_FALSE; /* free name when not needed */ inter_sp = sp; /* this is needed for assert_master_ob_loaded(), and for * the possible errors before. @@ -3302,12 +3303,14 @@ if (name[length-2] == '.' && name[length-1] == 'c') { /* A new writeable copy of the name is needed. */ char *p; - p = (char *)alloca(length+1); + p = xalloc(length+1); strcpy(p, name); name = p; name[length -= 2] = '\0'; + freenamebuffer = MY_TRUE; } + /* check for any #xxx at the end. */ { char c; char *p; @@ -3328,18 +3331,38 @@ } } + m_name = new_mstring(name); + if (!m_name) + { + if (freenamebuffer) + xfree(name); + errorf("Out of memory for object name (%zu bytes)\n", strlen(name)); + } + /* in case of errors (e.g. in privilege_violation()), push string on the + * stack. */ + push_string(sp, m_name); + inter_sp = sp; + + /* name is not needed anymore. Free it, if it was allocated here. */ + if (freenamebuffer) + { + xfree(name); + /* just to be sure it crashes if somebody uses the pointer by + * accident. */ + name = NULL; + } + assert_master_ob_loaded(); if (master_ob == ob) + { errorf("Attempt to rename the master object\n"); + } - m_name = new_mstring(name); - if (!m_name) - errorf("Out of memory for object name (%zu bytes)\n", strlen(name)); - + if (lookup_object_hash(m_name)) { - free_mstring(m_name); - errorf("Attempt to rename to existing object '%s'\n", name); + errorf("Attempt to rename to existing object '%s'\n", + get_txt(m_name)); } if (privilege_violation4(STR_RENAME_OBJECT, ob, m_name, 0, sp) @@ -3351,11 +3374,10 @@ ob->name = m_name; enter_object_hash(ob); } - else - free_mstring(m_name); - free_svalue(sp--); - free_svalue(sp--); + /* free the string m_name (on the top of the stack) and the 2 arguments */ + sp = pop_n_elems(3, sp); + return sp; } /* f_rename_object() */ @@ -3404,6 +3426,7 @@ errorf("replace_program called with no current object\n"); if (current_object == simul_efun_object) errorf("replace_program on simul_efun object\n"); + if (current_object->flags & O_LAMBDA_REFERENCED) { inter_sp = sp; @@ -3412,11 +3435,7 @@ , get_txt(current_object->name) , get_txt(current_prog->name) ); - for ( ; num_arg != 0; num_arg--) - { - free_svalue(sp); - sp--; - } + sp = pop_n_elems(num_arg, sp); return sp; } @@ -3463,21 +3482,39 @@ else { string_t *sname; - long name_len; - char *name; + { /* block for limiting the scope of name until the xfree. */ + size_t name_len; + char *name; - /* Create the full program name with a trailing '.c' and without - * a leading '/' to match the internal name representation. - */ - name_len = (long)mstrsize(sp->u.str); - name = alloca((size_t)name_len+3); - strcpy(name, get_txt(sp->u.str)); - if (name[name_len-2] != '.' || name[name_len-1] != 'c') - strcat(name,".c"); - if (*name == '/') - name++; - memsafe(sname = new_mstring(name), name_len+3, "normalized program name"); - + /* Create the full program name with a trailing '.c' and without + * a leading '/' to match the internal name representation. + */ + name_len = mstrsize(sp->u.str); + name = xalloc(name_len+3); + if (!name) + { + errorf("Out of memory (%zu bytes) for temporary name buffer in " + "replace_program.\n", name_len); + } + strcpy(name, get_txt(sp->u.str)); + if (name[name_len-2] != '.' || name[name_len-1] != 'c') + strcat(name,".c"); + if (*name == '/') + sname = new_mstring(name+1); + else + sname = new_mstring(name); + /* name is not needed anymore, free it first, before throwing any + * runtime errors. */ + xfree(name); + + /* now check if we got a string from new_mstring(). */ + if (!sname) + { + errorf("Out of memory (%zu bytes) for temporary name in " + "replace_program().\n", name_len+3); + } + } /* scope of name ends here */ + new_prog = search_inherited(sname, current_object->prog, offsets); if (!new_prog) { | ||||
child of | 0000545 | new | Usages of alloca() have to be checked for possible stack overflow |
Date Modified | Username | Field | Change |
---|---|---|---|
2008-10-01 15:13 | zesstra | New Issue | |
2008-10-01 15:13 | zesstra | Status | new => assigned |
2008-10-01 15:13 | zesstra | Assigned To | => zesstra |
2008-10-01 15:13 | zesstra | Relationship added | child of 0000545 |
2008-10-04 14:54 | zesstra | File Added: objects.diff | |
2008-10-04 14:55 | zesstra | Note Added: 0000802 | |
2008-12-15 11:16 | zesstra | Status | assigned => resolved |
2008-12-15 11:16 | zesstra | Fixed in Version | => 3.3.718 |
2008-12-15 11:16 | zesstra | Resolution | open => fixed |
2008-12-15 11:16 | zesstra | Note Added: 0000822 | |
2010-11-16 09:42 | zesstra | Source_changeset_attached | => ldmud.git master 2d8bacb9 |
2018-01-29 18:59 | zesstra | Source_changeset_attached | => ldmud.git master 2d8bacb9 |
2018-01-29 21:57 | zesstra | Source_changeset_attached | => ldmud.git master 2d8bacb9 |