From: Arran Cudbard-Bell Date: Sun, 29 Nov 2020 22:59:38 +0000 (-0700) Subject: Check for errors using out, not slen X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=5f39d66db71f8e769fe3237045c0d4d381ff432c;p=thirdparty%2Ffreeradius-server.git Check for errors using out, not slen --- diff --git a/src/lib/server/cf_parse.c b/src/lib/server/cf_parse.c index ed4cd5b1c21..52573105ec2 100644 --- a/src/lib/server/cf_parse.c +++ b/src/lib/server/cf_parse.c @@ -247,7 +247,7 @@ int cf_pair_parse_value(TALLOC_CTX *ctx, void *out, UNUSED void *base, CONF_ITEM slen = tmpl_afrom_substr(cp, &vpt, &sbuff, cp->rhs_quote, tmpl_parse_rules_unquoted[cp->rhs_quote], &rules); - if (slen < 0) goto tmpl_error; + if (!vpt) goto tmpl_error; if (attribute && (!tmpl_is_attr(vpt) && !tmpl_is_attr_unresolved(vpt))) { cf_log_err(cp, "Expected attr got %s", @@ -1360,6 +1360,8 @@ static int cf_parse_tmpl_pass2(UNUSED CONF_SECTION *cs, tmpl_t **out, CONF_PAIR { tmpl_t *vpt = *out; + fr_assert(vpt); /* We need something to resolve */ + if (tmpl_resolve(vpt) < 0) { cf_log_perr(cp, "Failed processing configuration item '%s'", cp->attr); return -1; diff --git a/src/lib/server/tmpl_tokenize.c b/src/lib/server/tmpl_tokenize.c index c0c947d64b2..7ccb96b728f 100644 --- a/src/lib/server/tmpl_tokenize.c +++ b/src/lib/server/tmpl_tokenize.c @@ -2384,7 +2384,8 @@ static ssize_t tmpl_afrom_integer_substr(TALLOC_CTX *ctx, tmpl_t **out, fr_sbuff /** Convert an arbitrary string into a #tmpl_t * * @note Unlike #tmpl_afrom_attr_str return code 0 doesn't necessarily indicate failure, - * may just mean a 0 length string was parsed. + * may just mean a 0 length string was parsed. Check to see if the function emitted + * a #tmpl_t in *out. * * @note xlats and regexes are left uncompiled. This is to support the two pass parsing * done by the modcall code. Compilation on pass1 of that code could fail, as diff --git a/src/lib/unlang/compile.c b/src/lib/unlang/compile.c index 54fa6ec38e9..928e9e1d3b5 100644 --- a/src/lib/unlang/compile.c +++ b/src/lib/unlang/compile.c @@ -421,7 +421,7 @@ static bool pass2_fixup_map(fr_cond_t *c) &(tmpl_rules_t){ .allow_unknown = true }); - if (slen < 0) { + if (!vpt) { char *spaces, *text; fr_canonicalize_error(map->ci, &spaces, &text, slen, fr_strerror()); @@ -1299,7 +1299,7 @@ static unlang_t *compile_map(unlang_t *parent, unlang_compile_t *unlang_ctx, CON type, NULL, &parse_rules); - if (slen < 0) { + if (!vpt) { cf_log_perr(cs, "Failed parsing map"); error: talloc_free(g); @@ -1964,7 +1964,7 @@ static unlang_t *compile_switch(UNUSED unlang_t *parent, unlang_compile_t *unlan type, NULL, &parse_rules); - if (slen < 0) { + if (!gext->vpt) { char *spaces, *text; fr_canonicalize_error(cs, &spaces, &text, slen, fr_strerror()); @@ -2109,7 +2109,7 @@ static unlang_t *compile_case(unlang_t *parent, unlang_compile_t *unlang_ctx, CO type, NULL, &parse_rules); - if (slen < 0) { + if (!vpt) { char *spaces, *text; fr_canonicalize_error(cs, &spaces, &text, slen, fr_strerror()); @@ -2250,7 +2250,7 @@ static unlang_t *compile_foreach(unlang_t *parent, unlang_compile_t *unlang_ctx, type, NULL, &parse_rules); - if (slen <= 0) { + if (!vpt) { char *spaces, *text; fr_canonicalize_error(cs, &spaces, &text, slen, fr_strerror()); @@ -2423,7 +2423,7 @@ static unlang_t *compile_tmpl(unlang_t *parent, cf_pair_attr_quote(cp), NULL, unlang_ctx->rules); - if (slen <= 0) { + if (!vpt) { char *spaces, *text; fr_canonicalize_error(cp, &spaces, &text, slen, fr_strerror()); @@ -2678,7 +2678,7 @@ static unlang_t *compile_load_balance_subsection(unlang_t *parent, unlang_compil type, NULL, &parse_rules); - if (slen < 0) { + if (!gext->vpt) { char *spaces, *text; fr_canonicalize_error(cs, &spaces, &text, slen, fr_strerror()); @@ -2953,7 +2953,7 @@ get_packet_type: slen = tmpl_afrom_substr(parent, &src_vpt, &FR_SBUFF_IN(src, talloc_array_length(src) - 1), cf_section_argv_quote(cs, 0), NULL, unlang_ctx->rules); - if (slen <= 0) { + if (!src_vpt) { cf_log_perr(cs, "Invalid argument to 'subrequest', failed parsing src"); error: talloc_free(vpt); @@ -2972,7 +2972,7 @@ get_packet_type: slen = tmpl_afrom_substr(parent, &dst_vpt, &FR_SBUFF_IN(dst, talloc_array_length(dst) - 1), cf_section_argv_quote(cs, 1), NULL, unlang_ctx->rules); - if (slen <= 0) { + if (!dst_vpt) { cf_log_perr(cs, "Invalid argument to 'subrequest', failed parsing dst"); goto error; } diff --git a/src/modules/rlm_exec/rlm_exec.c b/src/modules/rlm_exec/rlm_exec.c index 4cce02d684c..b34dd74b04d 100644 --- a/src/modules/rlm_exec/rlm_exec.c +++ b/src/modules/rlm_exec/rlm_exec.c @@ -229,14 +229,15 @@ static int mod_instantiate(void *instance, CONF_SECTION *conf) if (!inst->program) return 0; - slen = tmpl_afrom_substr(inst, &inst->tmpl, &FR_SBUFF_IN(inst->program, strlen(inst->program)), + slen = tmpl_afrom_substr(inst, &inst->tmpl, + &FR_SBUFF_IN(inst->program, strlen(inst->program)), T_BACK_QUOTED_STRING, NULL, &(tmpl_rules_t) { .allow_foreign = true, .allow_unresolved = false, .allow_unknown = false }); - if (slen <= 0) { + if (!inst->tmpl) { char *spaces, *text; fr_canonicalize_error(inst, &spaces, &text, slen, inst->program); diff --git a/src/modules/rlm_linelog/rlm_linelog.c b/src/modules/rlm_linelog/rlm_linelog.c index 992f79a7895..8a8fae7457d 100644 --- a/src/modules/rlm_linelog/rlm_linelog.c +++ b/src/modules/rlm_linelog/rlm_linelog.c @@ -530,7 +530,7 @@ static unlang_action_t CC_HINT(nonnull) mod_do_linelog(rlm_rcode_t *p_result, mo .allow_unresolved = false, .at_runtime = true }); - if (slen <= 0) { + if (!vpt) { REMARKER(tmpl_str, -slen, "%s", fr_strerror()); RETURN_MODULE_FAIL; } diff --git a/src/modules/rlm_smtp/rlm_smtp.c b/src/modules/rlm_smtp/rlm_smtp.c index bf3a1e6d1e7..1cea3348da2 100644 --- a/src/modules/rlm_smtp/rlm_smtp.c +++ b/src/modules/rlm_smtp/rlm_smtp.c @@ -130,7 +130,7 @@ static int cf_table_parse_tmpl(UNUSED TALLOC_CTX *ctx, void *out, UNUSED void *p &rules); /* There was an error */ - if (slen < 0) { + if (!vpt) { char *spaces, *text; fr_canonicalize_error(ctx, &spaces, &text, slen, cp->value); diff --git a/src/modules/rlm_winbind/rlm_winbind.c b/src/modules/rlm_winbind/rlm_winbind.c index c975ffa073b..c50a13c7828 100644 --- a/src/modules/rlm_winbind/rlm_winbind.c +++ b/src/modules/rlm_winbind/rlm_winbind.c @@ -424,6 +424,11 @@ static int mod_instantiate(void *instance, CONF_SECTION *conf) .allow_unknown = true, .allow_unresolved = true }); + if (!inst->wb_domain) { + cf_log_perr(conf, "Bad domain"); + wbcFreeMemory(wb_info); + return -1; + } cf_log_err(conf, "Using winbind_domain '%s'", inst->wb_domain->name); diff --git a/src/tests/unit/xlat/base.txt b/src/tests/unit/xlat/base.txt index ce6f093f729..9fc7358d898 100644 --- a/src/tests/unit/xlat/base.txt +++ b/src/tests/unit/xlat/base.txt @@ -263,6 +263,10 @@ match ERROR offset 19: Missing closing brace xlat %{myfirstxlat match ERROR offset 13: Missing closing brace +# Issue seen in the wild that caused an SEGV during pass2 +xlat %{%{control.IP-Pool.Name}:%{reply.IP-Pool.Range} +match ERROR offset 25: Expected ':-' after first expansion + # # API to split xlat strings into argv-style arguments. # @@ -297,4 +301,4 @@ xlat_argv echo hello %{Tmp-String-0}:1234 world match [0]{ echo }, [1]{ hello }, [2]{ %{Tmp-String-0}:1234 }, [3]{ world } count -match 177 +match 179