[sr-dev] git:master:2ecf601c: core: variables declared in the config file could cause memory corruption

[mtirpak]

Module: kamailio
Branch: master
Commit: 2ecf601c472bb81b9cf4ffd5b1ac17c4dfd742f2
URL: https://github.com/kamailio/kamailio/commit/2ecf601c472bb81b9cf4ffd5b1ac17c4dfd742f2

Author: mtirpak <miklos.tirpak at gmail.com>
Committer: mtirpak <miklos.tirpak at gmail.com>
Date: 2018-10-25T13:57:17+02:00

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.

---

Modified: src/core/cfg/cfg_script.c

---

Diff:  https://github.com/kamailio/kamailio/commit/2ecf601c472bb81b9cf4ffd5b1ac17c4dfd742f2.diff
Patch: https://github.com/kamailio/kamailio/commit/2ecf601c472bb81b9cf4ffd5b1ac17c4dfd742f2.patch

---

diff --git a/src/core/cfg/cfg_script.c b/src/core/cfg/cfg_script.c
index b85d103696..9873f70405 100644
--- a/src/core/cfg/cfg_script.c
+++ b/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