From: Arran Cudbard-Bell Date: Sat, 23 May 2020 15:49:48 +0000 (-0500) Subject: Temporarily revert changes to remove strsteal functions X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=68524b8edd5387d4e19c536809a36305903e1d40;p=thirdparty%2Ffreeradius-server.git Temporarily revert changes to remove strsteal functions --- diff --git a/src/lib/unlang/xlat_builtin.c b/src/lib/unlang/xlat_builtin.c index 6689c29488e..0ae318eedf2 100644 --- a/src/lib/unlang/xlat_builtin.c +++ b/src/lib/unlang/xlat_builtin.c @@ -1647,8 +1647,12 @@ static xlat_action_t xlat_func_base64_encode(TALLOC_CTX *ctx, fr_cursor_t *out, fr_assert((size_t)elen <= alen); MEM(vb = fr_value_box_alloc_null(ctx)); - (void) fr_value_box_bstrndup(vb, vb, NULL, buff, elen, (*in)->tainted); - talloc_free(buff); + + if (fr_value_box_bstrsnteal(vb, vb, NULL, &buff, elen, (*in)->tainted) < 0) { + RPEDEBUG("Failed assigning encoded data buffer to box"); + talloc_free(vb); + return XLAT_ACTION_FAIL; + } fr_cursor_append(out, vb); @@ -1786,7 +1790,7 @@ static xlat_action_t xlat_func_concat(TALLOC_CTX *ctx, fr_cursor_t *out, REQUEST *request, UNUSED void const *xlat_inst, UNUSED void *xlat_thread_inst, fr_value_box_t **in) { - fr_value_box_t *vb; + fr_value_box_t *result; fr_value_box_t *separator; char *buff; char const *sep; @@ -1808,12 +1812,19 @@ static xlat_action_t xlat_func_concat(TALLOC_CTX *ctx, fr_cursor_t *out, sep = separator->vb_strvalue; - MEM(vb = fr_value_box_alloc_null(ctx)); - MEM(buff = fr_value_box_list_asprint(vb, (*in)->next, sep, '\0')); + result = fr_value_box_alloc_null(ctx); + if (!result) { + error: + RPEDEBUG("Failed concatenating input"); + return XLAT_ACTION_FAIL; + } - fr_value_box_bstrassign(vb, NULL, buff, fr_value_box_list_tainted((*in)->next)); + buff = fr_value_box_list_asprint(result, (*in)->next, sep, '\0'); + if (!buff) goto error; - fr_cursor_append(out, vb); + fr_value_box_bstrsteal(result, result, NULL, buff, fr_value_box_list_tainted((*in)->next)); + + fr_cursor_append(out, result); return XLAT_ACTION_DONE; } @@ -1850,10 +1861,9 @@ static xlat_action_t xlat_func_hex(TALLOC_CTX *ctx, fr_cursor_t *out, return XLAT_ACTION_FAIL; } - MEM(vb = fr_value_box_alloc_null(ctx)); - p = talloc_zero_array(vb, char, ((*in)->vb_length * 2) + 1); - (void) fr_value_box_bstrassign(vb, NULL, p, false); - + MEM(vb = fr_value_box_alloc(ctx, FR_TYPE_STRING, NULL, false)); + vb->vb_length = ((*in)->vb_length * 2); + vb->vb_strvalue = p = talloc_zero_array(vb, char, vb->vb_length + 1); fr_bin2hex(p, (*in)->vb_octets, (*in)->vb_length); fr_cursor_append(out, vb); @@ -2209,13 +2219,14 @@ static xlat_action_t xlat_func_pairs(TALLOC_CTX *ctx, fr_cursor_t *out, fr_token_t op = vp->op; char *buff; - MEM(vb = fr_value_box_alloc_null(ctx)); + MEM(vb = fr_value_box_alloc(ctx, FR_TYPE_STRING, NULL, false)); vp->op = T_OP_EQ; buff = fr_pair_asprint(vb, vp, '"'); vp->op = op; - (void) fr_value_box_bstrassign(vb, NULL, buff, false); + vb->vb_strvalue = buff; + vb->vb_length = talloc_array_length(buff) - 1; fr_cursor_append(out, vb); } @@ -2359,9 +2370,7 @@ static xlat_action_t xlat_func_randstr(TALLOC_CTX *ctx, fr_cursor_t *out, p++; } - MEM(vb = fr_value_box_alloc_null(ctx)); - buff = buff_p = talloc_array(vb, char, outlen + 1); - (void) fr_value_box_bstrassign(vb, NULL, buff, false); + buff = buff_p = talloc_array(NULL, char, outlen + 1); /* Reset p to start position */ p = start; @@ -2456,7 +2465,7 @@ static xlat_action_t xlat_func_randstr(TALLOC_CTX *ctx, fr_cursor_t *out, default: REDEBUG("Invalid character class '%c'", *p); - talloc_free(vb); + talloc_free(buff); return XLAT_ACTION_FAIL; } @@ -2467,6 +2476,9 @@ static xlat_action_t xlat_func_randstr(TALLOC_CTX *ctx, fr_cursor_t *out, *buff_p++ = '\0'; + MEM(vb = fr_value_box_alloc(ctx, FR_TYPE_STRING, NULL, false)); + fr_value_box_bstrsteal(vb, vb, NULL, buff, false); + fr_cursor_append(out, vb); return XLAT_ACTION_DONE; @@ -2496,16 +2508,15 @@ static xlat_action_t xlat_func_regex(TALLOC_CTX *ctx, fr_cursor_t *out, fr_value_box_t *vb; char *p; - MEM(vb = fr_value_box_alloc_null(ctx)); - if (regex_request_to_sub(vb, &p, request, 0) < 0) { - talloc_free(vb); + if (regex_request_to_sub(ctx, &p, request, 0) < 0) { REDEBUG2("No previous regex capture"); return XLAT_ACTION_FAIL; } fr_assert(p); - (void) fr_value_box_bstrassign(vb, NULL, p, false); + MEM(vb = fr_value_box_alloc_null(ctx)); + fr_value_box_bstrsteal(vb, vb, NULL, p, false); fr_cursor_append(out, vb); return XLAT_ACTION_DONE; @@ -2532,16 +2543,15 @@ static xlat_action_t xlat_func_regex(TALLOC_CTX *ctx, fr_cursor_t *out, return XLAT_ACTION_FAIL; } - MEM(vb = fr_value_box_alloc_null(ctx)); - if (regex_request_to_sub(vb, &p, request, idx.vb_uint32) < 0) { - talloc_free(vb); + if (regex_request_to_sub(ctx, &p, request, idx.vb_uint32) < 0) { REDEBUG2("No previous numbered regex capture group"); return XLAT_ACTION_FAIL; } fr_assert(p); - fr_value_box_bstrassign(vb, NULL, p, false); + MEM(vb = fr_value_box_alloc_null(ctx)); + fr_value_box_bstrsteal(vb, vb, NULL, p, false); fr_cursor_append(out, vb); return XLAT_ACTION_DONE; @@ -2560,16 +2570,15 @@ static xlat_action_t xlat_func_regex(TALLOC_CTX *ctx, fr_cursor_t *out, return XLAT_ACTION_FAIL; } - MEM(vb = fr_value_box_alloc_null(ctx)); - if (regex_request_to_sub_named(vb, &p, request, (*in)->vb_strvalue) < 0) { - talloc_free(vb); + if (regex_request_to_sub_named(request, &p, request, (*in)->vb_strvalue) < 0) { REDEBUG2("No previous named regex capture group"); return XLAT_ACTION_FAIL; } fr_assert(p); - fr_value_box_bstrassign(vb, NULL, p, false); + MEM(vb = fr_value_box_alloc_null(ctx)); + fr_value_box_bstrsteal(vb, vb, NULL, p, false); fr_cursor_append(out, vb); return XLAT_ACTION_DONE; @@ -2895,8 +2904,7 @@ static xlat_action_t xlat_func_sub_regex(TALLOC_CTX *ctx, fr_cursor_t *out, talloc_free(pattern); return XLAT_ACTION_FAIL; } - - fr_value_box_bstrassign(vb, NULL, buff, (*in)->tainted); + fr_value_box_bstrsteal(vb, vb, NULL, buff, (*in)->tainted); fr_cursor_append(out, vb); @@ -3102,8 +3110,7 @@ static xlat_action_t xlat_change_case(TALLOC_CTX *ctx, fr_cursor_t *out, p = (*in)->vb_strvalue; end = p + (*in)->vb_length; - MEM(vb = fr_value_box_alloc_null(ctx)); - buff = buff_p = talloc_array(vb, char, (*in)->vb_length + 1); + buff = buff_p = talloc_array(NULL, char, (*in)->vb_length + 1); while (p < end) { *(buff_p++) = upper ? toupper ((int) *(p++)) : tolower((int) *(p++)); @@ -3111,7 +3118,8 @@ static xlat_action_t xlat_change_case(TALLOC_CTX *ctx, fr_cursor_t *out, *buff_p = '\0'; - fr_value_box_bstrassign(vb, NULL, buff, false); + MEM(vb = fr_value_box_alloc(ctx, FR_TYPE_STRING, NULL, false)); + fr_value_box_bstrsteal(vb, vb, NULL, buff, false); fr_cursor_append(out, vb); @@ -3207,8 +3215,7 @@ static xlat_action_t xlat_func_urlquote(TALLOC_CTX *ctx, fr_cursor_t *out, p++; } - MEM(vb = fr_value_box_alloc_null(ctx)); - buff = buff_p = talloc_array(vb, char, outlen + 1); + buff = buff_p = talloc_array(NULL, char, outlen + 1); /* Reset p to start position */ p = (*in)->vb_strvalue; @@ -3237,7 +3244,8 @@ static xlat_action_t xlat_func_urlquote(TALLOC_CTX *ctx, fr_cursor_t *out, *buff_p = '\0'; - fr_value_box_bstrassign(vb, NULL, buff, false); + MEM(vb = fr_value_box_alloc(ctx, FR_TYPE_STRING, NULL, false)); + fr_value_box_bstrsteal(vb, vb, NULL, buff, false); fr_cursor_append(out, vb); @@ -3294,8 +3302,7 @@ static xlat_action_t xlat_func_urlunquote(TALLOC_CTX *ctx, fr_cursor_t *out, outlen++; } - MEM(vb = fr_value_box_alloc_null(ctx)); - buff = buff_p = talloc_array(vb, char, outlen + 1); + buff = buff_p = talloc_array(NULL, char, outlen + 1); /* Reset p to start position */ p = (*in)->vb_strvalue; @@ -3311,7 +3318,7 @@ static xlat_action_t xlat_func_urlunquote(TALLOC_CTX *ctx, fr_cursor_t *out, if (!(c1 = memchr(hextab, tolower(*++p), 16)) || !(c2 = memchr(hextab, tolower(*++p), 16))) { REMARKER((*in)->vb_strvalue, p - (*in)->vb_strvalue, "Non-hex char in %% sequence"); - talloc_free(vb); + talloc_free(buff); return XLAT_ACTION_FAIL; } @@ -3321,7 +3328,8 @@ static xlat_action_t xlat_func_urlunquote(TALLOC_CTX *ctx, fr_cursor_t *out, *buff_p = '\0'; - fr_value_box_bstrassign(vb, NULL, buff, false); + MEM(vb = fr_value_box_alloc(ctx, FR_TYPE_STRING, NULL, false)); + fr_value_box_bstrsteal(vb, vb, NULL, buff, false); fr_cursor_append(out, vb); diff --git a/src/lib/unlang/xlat_eval.c b/src/lib/unlang/xlat_eval.c index b0867c550a7..f3feb76d8ca 100644 --- a/src/lib/unlang/xlat_eval.c +++ b/src/lib/unlang/xlat_eval.c @@ -768,9 +768,8 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_cursor_t *out, result_str = talloc_typed_strdup(NULL, ""); } - MEM(value = fr_value_box_alloc_null(ctx)); if (node->xlat->buf_len > 0) { - str = talloc_array(value, char, node->xlat->buf_len); + str = talloc_array(ctx, char, node->xlat->buf_len); str[0] = '\0'; /* Be sure the string is \0 terminated */ } @@ -778,12 +777,12 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_cursor_t *out, node->fmt, fr_box_strvalue_len(result_str, talloc_array_length(result_str) - 1)); - slen = node->xlat->func.sync(value, &str, node->xlat->buf_len, + slen = node->xlat->func.sync(ctx, &str, node->xlat->buf_len, node->xlat->mod_inst, NULL, request, result_str); xlat_debug_log_expansion(request, *in, *result); if (slen < 0) { talloc_free(result_str); - talloc_free(value); + talloc_free(str); return XLAT_ACTION_FAIL; } if (slen == 0) { /* Zero length result */ @@ -793,14 +792,19 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_cursor_t *out, (void)talloc_get_type_abort(str, char); /* Check output buffer is sane */ /* - * Ensure that the string is the correct size. + * Shrink the buffer */ - if ((size_t) slen != (talloc_array_length(str) - 1)) { - MEM(str = talloc_bstr_realloc(value, str, (size_t)slen)); + if ((node->xlat->buf_len > 0) && (slen > 0)) { + MEM(str = talloc_bstr_realloc(ctx, str, (size_t)slen)); } - fr_value_box_bstrassign(value, NULL, str, false); - fr_cursor_append(out, value); + /* + * Fixup talloc lineage and assign the + * output of the function to a box. + */ + MEM(value = fr_value_box_alloc_null(ctx)); + fr_value_box_bstrsteal(value, value, NULL, str, false); + fr_cursor_append(out, value); /* Append the result of the expansion */ talloc_free(result_str); xlat_debug_log_result(request, value); } @@ -1020,19 +1024,13 @@ xlat_action_t xlat_frame_eval(TALLOC_CTX *ctx, fr_cursor_t *out, xlat_exp_t cons node->fmt); xlat_debug_log_expansion(request, node, NULL); - MEM(value = fr_value_box_alloc_null(ctx)); - slen = node->xlat->func.sync(value, &str, node->xlat->buf_len, node->xlat->mod_inst, + slen = node->xlat->func.sync(ctx, &str, node->xlat->buf_len, node->xlat->mod_inst, NULL, request, NULL); - if (slen < 0) { - talloc_free(value); - goto fail; - } - if (slen == 0) { - talloc_free(value); - continue; - } + if (slen < 0) goto fail; + if (slen == 0) continue; - fr_value_box_bstrassign(value, NULL, str, false); + MEM(value = fr_value_box_alloc_null(ctx)); + fr_value_box_bstrsteal(value, value, NULL, str, false); fr_cursor_append(out, value); fr_cursor_next(out); @@ -1073,14 +1071,15 @@ xlat_action_t xlat_frame_eval(TALLOC_CTX *ctx, fr_cursor_t *out, xlat_exp_t cons XLAT_DEBUG("** [%i] %s(regex) - %%{%s}", unlang_interpret_stack_depth(request), __FUNCTION__, node->fmt); + if (regex_request_to_sub(ctx, &str, request, node->regex_index) < 0) continue; + /* + * Above call strdups the capture data, so + * we just need to fix up the talloc lineage + * and box it. + */ MEM(value = fr_value_box_alloc_null(ctx)); - if (regex_request_to_sub(value, &str, request, node->regex_index) < 0) { - talloc_free(value); - continue; - } - - fr_value_box_bstrassign(value, NULL, str, false); + fr_value_box_bstrsteal(value, value, NULL, str, false); fr_cursor_append(out, value); } continue; diff --git a/src/lib/unlang/xlat_tokenize.c b/src/lib/unlang/xlat_tokenize.c index cc7346f3a1f..7eb9b4d921d 100644 --- a/src/lib/unlang/xlat_tokenize.c +++ b/src/lib/unlang/xlat_tokenize.c @@ -844,20 +844,6 @@ ssize_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_t **head, char const *in, s * --foo=%{Bar} */ while ((q < end) && !isspace((int) *q)) { - /* - * Things like: echo foo"bar" - * are forbidden. - * - * The Unix shell typically allows this - * kind of nonsense, but there's no reason - * for us to do so. - */ - if ((*q == '`') || (*q == '"') || (*q == '\'')) { - fr_strerror_printf("Unexpected quoted string"); - talloc_free(my_head); - return -(q - in); - } - if (*q == '%') { if (q[1] != '{') { slen = 2; diff --git a/src/lib/util/value.c b/src/lib/util/value.c index 0e8eb257416..0477a3188f4 100644 --- a/src/lib/util/value.c +++ b/src/lib/util/value.c @@ -3435,13 +3435,13 @@ int fr_value_box_strdup_buffer(TALLOC_CTX *ctx, fr_value_box_t *dst, fr_dict_att return 0; } - -/** Assign a nul terminated talloced buffer into a #fr_value_box_t +/** Steal a nul terminated talloced buffer into a specified ctx, and assign to a #fr_value_box_t * - * The buffer MUST be parented by the input value box. + * Steal a talloced nul terminated buffer, setting fields in the dst value box appropriately. * * The buffer must be \0 terminated, or an error will be returned. * + * @param[in] ctx to allocate any new buffers in. * @param[in] dst to assign new buffer to. * @param[in] enumv Aliases for values. * @param[in] src a talloced nul terminated buffer. @@ -3450,10 +3450,11 @@ int fr_value_box_strdup_buffer(TALLOC_CTX *ctx, fr_value_box_t *dst, fr_dict_att * - 0 on success. * - -1 on failure. */ -int fr_value_box_bstrassign(fr_value_box_t *dst, fr_dict_attr_t const *enumv, - char *src, bool tainted) +int fr_value_box_bstrsteal(TALLOC_CTX *ctx, fr_value_box_t *dst, fr_dict_attr_t const *enumv, + char *src, bool tainted) { size_t len; + char *str; len = talloc_array_length(src); if ((len == 0) || (src[len - 1] != '\0')) { @@ -3461,11 +3462,15 @@ int fr_value_box_bstrassign(fr_value_box_t *dst, fr_dict_attr_t const *enumv, return -1; } - fr_assert(dst == talloc_parent(src)); + str = talloc_steal(ctx, src); + if (!str) { + fr_strerror_printf("Failed stealing string buffer"); + return -1; + } dst->type = FR_TYPE_STRING; dst->tainted = tainted; - dst->vb_strvalue = src; + dst->vb_strvalue = str; dst->datum.length = len - 1; dst->enumv = enumv; dst->next = NULL; @@ -3473,6 +3478,50 @@ int fr_value_box_bstrassign(fr_value_box_t *dst, fr_dict_attr_t const *enumv, return 0; } +/** Steal a talloced string buffer into a specified ctx, reallocing if necessary + * + * @param[in] ctx to allocate any new buffers in. + * @param[in] dst to assign new buffer to. + * @param[in] enumv Aliases for values. + * @param[in] src a talloced buffer. + * @param[in] inlen length of data in the buffer. Buffer will be realloced if necessary. + * @param[in] tainted Whether the value came from a trusted source. + * @return + * - 0 on success. + * - -1 on failure. + */ +int fr_value_box_bstrsnteal(TALLOC_CTX *ctx, fr_value_box_t *dst, fr_dict_attr_t const *enumv, + char **src, size_t inlen, bool tainted) +{ + size_t len; + char *str; + + str = talloc_steal(ctx, *src); + if (!str) { + fr_strerror_printf("Failed stealing string buffer"); + return -1; + } + + len = talloc_array_length(*src); + if (len != (inlen + 1)) { /* +1 for \0 */ + str = talloc_realloc_size(ctx, *src, inlen + 1); + if (!str) { + fr_strerror_printf("Failed reallocing string buffer"); + return -1; + } + str[inlen] = '\0'; + *src = str; + } + + dst->type = FR_TYPE_STRING; + dst->tainted = tainted; + dst->vb_strvalue = str; + dst->datum.length = inlen; + dst->enumv = enumv; + dst->next = NULL; + + return 0; +} /** Append a buffer to an existing fr_value_box_t * diff --git a/src/lib/util/value.h b/src/lib/util/value.h index 9d45a9ba5e3..cf2a6c59bac 100644 --- a/src/lib/util/value.h +++ b/src/lib/util/value.h @@ -599,8 +599,10 @@ int fr_value_box_bstrndup(TALLOC_CTX *ctx, fr_value_box_t *dst, fr_dict_attr_t void fr_value_box_bstrndup_shallow(fr_value_box_t *dst, fr_dict_attr_t const *enumv, char const *src, size_t len, bool tainted); -int fr_value_box_bstrassign(fr_value_box_t *dst, fr_dict_attr_t const *enumv, +int fr_value_box_bstrsteal(TALLOC_CTX *ctx, fr_value_box_t *dst, fr_dict_attr_t const *enumv, char *src, bool tainted); +int fr_value_box_bstrsnteal(TALLOC_CTX *ctx, fr_value_box_t *dst, fr_dict_attr_t const *enumv, + char **src, size_t inlen, bool tainted); int fr_value_box_append_bstr(TALLOC_CTX *ctx, fr_value_box_t *dst, char const *src, size_t len, bool tainted); diff --git a/src/modules/rlm_cipher/rlm_cipher.c b/src/modules/rlm_cipher/rlm_cipher.c index a973dc59a90..a0e1526db9f 100644 --- a/src/modules/rlm_cipher/rlm_cipher.c +++ b/src/modules/rlm_cipher/rlm_cipher.c @@ -653,7 +653,7 @@ static xlat_action_t cipher_rsa_decrypt_xlat(TALLOC_CTX *ctx, fr_cursor_t *out, uint8_t const *ciphertext; size_t ciphertext_len; - char *plaintext, *n; + char *plaintext; size_t plaintext_len; fr_value_box_t *vb; @@ -682,7 +682,6 @@ static xlat_action_t cipher_rsa_decrypt_xlat(TALLOC_CTX *ctx, fr_cursor_t *out, MEM(plaintext = talloc_array(ctx, char, plaintext_len + 1)); if (EVP_PKEY_decrypt(xt->evp_decrypt_ctx, (unsigned char *)plaintext, &plaintext_len, ciphertext, ciphertext_len) <= 0) { - talloc_free(plaintext); fr_tls_log_error(request, "Failed decrypting ciphertext"); return XLAT_ACTION_FAIL; } @@ -691,17 +690,21 @@ static xlat_action_t cipher_rsa_decrypt_xlat(TALLOC_CTX *ctx, fr_cursor_t *out, /* * Fixup the output buffer (and ensure it's \0 terminated) */ - MEM(vb = fr_value_box_alloc_null(ctx)); - n = talloc_bstrndup(vb, plaintext, plaintext_len); - if (unlikely(!n)) { - REDEBUG("Failed shrinking plaintext buffer"); - talloc_free(vb); - talloc_free(plaintext); - return XLAT_ACTION_FAIL; + { + char *n; + + n = talloc_bstr_realloc(ctx, plaintext, plaintext_len); + if (unlikely(!n)) { + REDEBUG("Failed shrinking plaintext buffer"); + talloc_free(plaintext); + return XLAT_ACTION_FAIL; + } + + plaintext = n; } - talloc_free(plaintext); - fr_value_box_bstrassign(vb, NULL, n, false); + MEM(vb = fr_value_box_alloc_null(ctx)); + fr_value_box_bstrsteal(vb, vb, NULL, plaintext, false); fr_cursor_append(out, vb); return XLAT_ACTION_DONE; diff --git a/src/modules/rlm_json/rlm_json.c b/src/modules/rlm_json/rlm_json.c index 5fd85a81132..613f1129890 100644 --- a/src/modules/rlm_json/rlm_json.c +++ b/src/modules/rlm_json/rlm_json.c @@ -120,7 +120,7 @@ static xlat_action_t json_quote_xlat(TALLOC_CTX *ctx, fr_cursor_t *out, REQUEST return XLAT_ACTION_FAIL; } - if (unlikely(fr_value_box_bstrassign(vb, NULL, tmp, false) < 0)) { + if (unlikely(fr_value_box_bstrsteal(vb, vb, NULL, tmp, false) < 0)) { REDEBUG("Failed to allocate JSON string"); goto error; } @@ -294,8 +294,7 @@ static xlat_action_t json_encode_xlat(TALLOC_CTX *ctx, fr_cursor_t *out, REQUEST goto error; } - if (unlikely(fr_value_box_bstrassign(vb, NULL, json_str, false) < 0)) { - talloc_free(vb); + if (unlikely(fr_value_box_bstrsteal(vb, vb, NULL, json_str, false) < 0)) { REDEBUG("Failed to allocate JSON string"); goto error; }