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 |