From fde6a66472259d950ad6790d400d476b8f70a7cc Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Wed, 16 Oct 2019 18:55:14 +0200 Subject: [PATCH] Fix wrong config-unhexing if %25 (%) occurs --- ChangeLog | 6 +++- gconfig.c | 63 ++++++++++++++++++++++------------------ gconfig.h | 2 ++ radsecproxy.c | 12 ++++---- tests/t_rewrite_config.c | 6 ++-- 5 files changed, 51 insertions(+), 38 deletions(-) diff --git a/ChangeLog b/ChangeLog index 4b07ae7..3f75d7c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,8 @@ -2019-09-30 changes since 1.8.0 +chanes since 1.8.1 + Bug fixes: + - Fix wrong config-unhexing if %25 (%) occurs + +2019-10-01 1.8.1 Bug fixes: - Handle Tunnel-Password attribute correctly - Fix BSD platform issues diff --git a/gconfig.c b/gconfig.c index 81fe63e..db60501 100644 --- a/gconfig.c +++ b/gconfig.c @@ -415,12 +415,14 @@ int getgenericconfig(struct gconffile **cf, char *block, ...) { while ((word = va_arg(ap, char *))) { type = va_arg(ap, int); switch (type) { - case CONF_STR: + case CONF_STR: /*intentional fall-thru, these are identical*/ + case CONF_STR_NOESC: str = va_arg(ap, char **); if (!str) goto errparam; break; - case CONF_MSTR: + case CONF_MSTR: /*intentional fall-thru, these are identical*/ + case CONF_MSTR_NOESC: mstr = va_arg(ap, char ***); if (!mstr) goto errparam; @@ -456,38 +458,43 @@ int getgenericconfig(struct gconffile **cf, char *block, ...) { goto errexit; } - if (((type == CONF_STR || type == CONF_MSTR || type == CONF_BLN || type == CONF_LINT) && conftype != CONF_STR) || - (type == CONF_CBK && conftype != CONF_CBK)) { + if (((type == CONF_STR || type == CONF_STR_NOESC || type == CONF_MSTR || type == CONF_MSTR_NOESC || + type == CONF_BLN || type == CONF_LINT) && conftype != CONF_STR) || + (type == CONF_CBK && conftype != CONF_CBK)) { if (block) debug(DBG_ERR, "configuration error in block %s, wrong syntax for option %s", block, opt); debug(DBG_ERR, "configuration error, wrong syntax for option %s", opt); goto errexit; } - switch (type) { - case CONF_STR: - if (*str) { - debug(DBG_ERR, "configuration error, option %s already set to %s", opt, *str); - goto errexit; - } - unhex(val,0); - *str = val; - break; - case CONF_MSTR: - if (*mstr) - for (n = 0; (*mstr)[n]; n++); - else - n = 0; - newmstr = realloc(*mstr, sizeof(char *) * (n + 2)); - if (!newmstr) { - debug(DBG_ERR, "malloc failed"); - goto errexit; - } - unhex(val,0); - newmstr[n] = val; - newmstr[n + 1] = NULL; - *mstr = newmstr; - break; + switch (type) { + case CONF_STR: /*intentional fall-thru, these are almost identical*/ + case CONF_STR_NOESC: + if (*str) { + debug(DBG_ERR, "configuration error, option %s already set to %s", opt, *str); + goto errexit; + } + if (type == CONF_STR) + unhex(val,0); + *str = val; + break; + case CONF_MSTR: /*intentional fall-thru, these are almost identical*/ + case CONF_MSTR_NOESC: + if (*mstr) + for (n = 0; (*mstr)[n]; n++); + else + n = 0; + newmstr = realloc(*mstr, sizeof(char *) * (n + 2)); + if (!newmstr) { + debug(DBG_ERR, "malloc failed"); + goto errexit; + } + if (type == CONF_MSTR) + unhex(val,0); + newmstr[n] = val; + newmstr[n + 1] = NULL; + *mstr = newmstr; + break; case CONF_BLN: if (!strcasecmp(val, "on")) *bln = 1; diff --git a/gconfig.h b/gconfig.h index a5276f1..6657d96 100644 --- a/gconfig.h +++ b/gconfig.h @@ -6,6 +6,8 @@ #define CONF_MSTR 3 #define CONF_BLN 4 #define CONF_LINT 5 +#define CONF_STR_NOESC 6 +#define CONF_MSTR_NOESC 7 #include diff --git a/radsecproxy.c b/radsecproxy.c index 76b9f90..d5204db 100644 --- a/radsecproxy.c +++ b/radsecproxy.c @@ -2322,7 +2322,7 @@ int confclient_cb(struct gconffile **cf, void *arg, char *block, char *opt, char "host", CONF_MSTR, &conf->hostsrc, "IPv4Only", CONF_BLN, &ipv4only, "IPv6Only", CONF_BLN, &ipv6only, - "secret", CONF_STR, &conf->confsecret, + "secret", CONF_STR_NOESC, &conf->confsecret, #if defined(RADPROT_TLS) || defined(RADPROT_DTLS) "tls", CONF_STR, &conf->tls, "matchcertificateattribute", CONF_STR, &conf->matchcertattr, @@ -2523,7 +2523,7 @@ int confserver_cb(struct gconffile **cf, void *arg, char *block, char *opt, char "IPv4Only", CONF_BLN, &ipv4only, "IPv6Only", CONF_BLN, &ipv6only, "port", CONF_STR, &conf->portsrc, - "secret", CONF_STR, &conf->confsecret, + "secret", CONF_STR_NOESC, &conf->confsecret, #if defined(RADPROT_TLS) || defined(RADPROT_DTLS) "tls", CONF_STR, &conf->tls, "MatchCertificateAttribute", CONF_STR, &conf->matchcertattr, @@ -2688,12 +2688,12 @@ int confrewrite_cb(struct gconffile **cf, void *arg, char *block, char *opt, cha "removeVendorAttribute", CONF_MSTR, &rmvattrs, "whitelistAttribute", CONF_MSTR, &wlattrs, "whitelistVendorAttribute", CONF_MSTR, &wlvattrs, - "addAttribute", CONF_MSTR, &addattrs, - "addVendorAttribute", CONF_MSTR, &addvattrs, + "addAttribute", CONF_MSTR_NOESC, &addattrs, + "addVendorAttribute", CONF_MSTR_NOESC, &addvattrs, "modifyAttribute", CONF_MSTR, &modattrs, "modifyVendorAttribute", CONF_MSTR, &modvattrs, - "supplementAttribute", CONF_MSTR, &supattrs, - "supplementVendorAttribute", CONF_MSTR, &supvattrs, + "supplementAttribute", CONF_MSTR_NOESC, &supattrs, + "supplementVendorAttribute", CONF_MSTR_NOESC, &supvattrs, NULL)) debugx(1, DBG_ERR, "configuration error"); addrewrite(val, whitelist_mode, whitelist_mode? wlattrs : rmattrs, whitelist_mode? wlvattrs : rmvattrs, diff --git a/tests/t_rewrite_config.c b/tests/t_rewrite_config.c index 97ae7fc..2e0ac89 100644 --- a/tests/t_rewrite_config.c +++ b/tests/t_rewrite_config.c @@ -17,16 +17,16 @@ main (int argc, char *argv[]) char **addattrs; int numtests = 1, i; struct tlv *tlv, *expected; - uint8_t expectedvalue[] = {'1',0,0,'1','A'}; + uint8_t expectedvalue[] = {'1',0,0,'1','A','%','4','1'}; printf("1..%d\n", numtests); numtests = 1; addattrs = malloc(2); - addattrs[0] = stringcopy("1:'1%00%001%41", 0); + addattrs[0] = stringcopy("1:'1%00%001%41%2541", 0); addattrs[1] = NULL; - expected = maketlv(1,5,expectedvalue); + expected = maketlv(1,8,expectedvalue); addrewrite(rewritename, 0, NULL, NULL, addattrs, NULL, NULL, NULL, NULL, NULL);