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

Module: kamailio Branch: master Commit: 2ecf601c472bb81b9cf4ffd5b1ac17c4dfd742f2 URL: https://github.com/kamailio/kamailio/commit/2ecf601c472bb81b9cf4ffd5b1ac17c… Author: mtirpak <miklos.tirpak(a)gmail.com> Committer: mtirpak <miklos.tirpak(a)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/2ecf601c472bb81b9cf4ffd5b1ac17c… Patch: https://github.com/kamailio/kamailio/commit/2ecf601c472bb81b9cf4ffd5b1ac17c… --- 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