Check for errors using out, not slen

diff --git a/src/lib/server/cf_parse.c b/src/lib/server/cf_parse.c
index ed4cd5b1c214f3eb0306043000e7697b0c9b42b7..52573105ec2aa301671e4cc30f0b1199a0696e79 100644 (file)
--- 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 c0c947d64b24bdc16bc2f3427968525aac484ab3..7ccb96b728fb0b7382160e4b93d7a78aa1f51abb 100644 (file)
--- 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 54fa6ec38e907a7fee72a1d8b33c371b5f180384..928e9e1d3b5ffca9ee66364196680d308d585759 100644 (file)
--- 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 4cce02d684cca13255946af54b0112519ed873c8..b34dd74b04d6ccc3d0232048d04b0a84e80b95b2 100644 (file)
--- 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 992f79a78954af39f0408dd8041b373e68d89b5f..8a8fae7457dd0a42ce6e5b9cdcd43a44c8941d8e 100644 (file)
--- 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 bf3a1e6d1e7ee3f5cf41a2fe629151286b01dd26..1cea3348da2fb76f23c5b4856c51fb7d2ea7aa23 100644 (file)
--- 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 c975ffa073b85739116114657ab26b6ece0ab582..c50a13c78284a5a0c3ebf9c0fafe8df16ad5b35a 100644 (file)
--- 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 ce6f093f72983601c41eb9fc7883b9ce6e7a5858..9fc7358d89873c27ad49b5652927d7be5a2f8417 100644 (file)
--- 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