Browse Source

core: variables declared in the config file could cause memory corruption

The config variables that are declared in the config file were recorded
in the reverse order as their padding was calculated, which could cause
the allocated memory block to be smaller as required at the end.

Credits go to vinesinha.
mtirpak 7 năm trước cách đây
mục cha
commit
2ecf601c47
1 tập tin đã thay đổi với 18 bổ sung4 xóa
  1. 18 4
      src/core/cfg/cfg_script.c

+ 18 - 4
src/core/cfg/cfg_script.c

@@ -35,7 +35,7 @@ cfg_script_var_t *new_cfg_script_var(char *gname, char *vname, unsigned int type
 					char *descr)
 {
 	cfg_group_t	*group;
-	cfg_script_var_t	*var;
+	cfg_script_var_t	*var, **last_var;
 	int	gname_len, vname_len, descr_len;
 
 	LM_DBG("declaring %s.%s\n", gname, vname);
@@ -112,9 +112,15 @@ cfg_script_var_t *new_cfg_script_var(char *gname, char *vname, unsigned int type
 	memset(var, 0, sizeof(cfg_script_var_t));
 	var->type = type;
 
-	/* add the variable to the group */
-	var->next = (cfg_script_var_t *)(void *)group->vars;
-	group->vars = (char *)(void *)var;
+	/* Add the variable to the end of the group.
+	 * The order is important because the padding depends on that.
+	 * The list will be travelled later again, which must be done in
+	 * the same order. */
+	last_var = (cfg_script_var_t **)(void **)&group->vars;
+	while ((*last_var))
+		last_var = &((*last_var)->next);
+	*last_var = var;
+	var->next = NULL;
 
 	/* clone the name of the variable */
 	var->name = (char *)pkg_malloc(sizeof(char) * (vname_len + 1));
@@ -282,6 +288,14 @@ int cfg_script_fixup(cfg_group_t *group, unsigned char *block)
 		}
 	}
 
+	/* Sanity check for the group size, make sure that the
+	 * newly calculated size equals the already calculated
+	 * group size. */
+	if (offset != group->size) {
+		LM_ERR("BUG: incorrect group size: %d; previously calculated value: %d \n", offset, group->size);
+		goto error;
+	}
+
 	/* allocate a handle even if it will not be used to
 	directly access the variable, like handle->variable
 	cfg_get_* functions access the memory block via the handle