From 338c670745632fede68b501552260bfbd0d2126f Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Fri, 14 Feb 2020 11:34:35 +0100 Subject: [PATCH] MEDIUM: arg: copy parsed arguments into the trash instead of allocating them For each and every argument parsed by make_arg_list(), there was an strndup() call, just so that we have a trailing zero for most functions, and this temporary buffer is released afterwards except for strings where it is kept. Proceeding like this is not convenient because 1) it performs a huge malloc/free dance, and 2) it forces to decide upfront where the argument ends, which is what prevents commas and right parenthesis from being used. This patch makes the function copy the temporary argument into the trash instead, so that we avoid the malloc/free dance for most all non-string args (e.g. integers, addresses, time, size etc), and that we can later produce the contents on the fly while parsing the input. It adds a length check to make sure that the argument is not longer than the buffer size, which should obviously never be the case but who knows what people put in their configuration. --- src/arg.c | 65 +++++++++++++++++++++++++++++-------------------------- 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/src/arg.c b/src/arg.c index ce4904621..927aaa4d0 100644 --- a/src/arg.c +++ b/src/arg.c @@ -15,6 +15,8 @@ #include #include +#include +#include #include const char *arg_type_names[ARGT_NBTYPES] = { @@ -111,7 +113,6 @@ int make_arg_list(const char *in, int len, uint64_t mask, struct arg **argp, int pos; struct arg *arg; const char *beg; - char *word = NULL; const char *ptr_err = NULL; int min_arg; int empty; @@ -163,17 +164,18 @@ int make_arg_list(const char *in, int len, uint64_t mask, struct arg **argp, * By default, the output argument will be the same type of the * expected one. */ - free(word); - word = my_strndup(beg, in - beg); + if (!chunk_strncpy(&trash, beg, in - beg)) + goto buffer_err; arg->type = (mask >> (pos * ARGT_BITS)) & ARGT_MASK; switch (arg->type) { case ARGT_SINT: - if (in == beg) // empty number + if (!trash.data) // empty number goto empty_err; - arg->data.sint = read_int64(&beg, in); - if (beg < in) + beg = trash.area; + arg->data.sint = read_int64(&beg, trash.area + trash.data); + if (beg < trash.area + trash.data) goto parse_err; arg->type = ARGT_SINT; break; @@ -196,56 +198,55 @@ int make_arg_list(const char *in, int len, uint64_t mask, struct arg **argp, * during the parsing. The caller must at one point resolve * them and free the string. */ - arg->data.str.area = word; - arg->data.str.data = in - beg; - arg->data.str.size = arg->data.str.data + 1; - word = NULL; + arg->data.str.area = my_strndup(trash.area, trash.data); + arg->data.str.data = trash.data; + arg->data.str.size = trash.data + 1; break; case ARGT_IPV4: - if (in == beg) // empty address + if (!trash.data) // empty address goto empty_err; - if (inet_pton(AF_INET, word, &arg->data.ipv4) <= 0) + if (inet_pton(AF_INET, trash.area, &arg->data.ipv4) <= 0) goto parse_err; break; case ARGT_MSK4: - if (in == beg) // empty mask + if (!trash.data) // empty mask goto empty_err; - if (!str2mask(word, &arg->data.ipv4)) + if (!str2mask(trash.area, &arg->data.ipv4)) goto parse_err; arg->type = ARGT_IPV4; break; case ARGT_IPV6: - if (in == beg) // empty address + if (!trash.data) // empty address goto empty_err; - if (inet_pton(AF_INET6, word, &arg->data.ipv6) <= 0) + if (inet_pton(AF_INET6, trash.area, &arg->data.ipv6) <= 0) goto parse_err; break; case ARGT_MSK6: - if (in == beg) // empty mask + if (!trash.data) // empty mask goto empty_err; - if (!str2mask6(word, &arg->data.ipv6)) + if (!str2mask6(trash.area, &arg->data.ipv6)) goto parse_err; arg->type = ARGT_IPV6; break; case ARGT_TIME: - if (in == beg) // empty time + if (!trash.data) // empty time goto empty_err; - ptr_err = parse_time_err(word, &uint, TIME_UNIT_MS); + ptr_err = parse_time_err(trash.area, &uint, TIME_UNIT_MS); if (ptr_err) { if (ptr_err == PARSE_TIME_OVER || ptr_err == PARSE_TIME_UNDER) - ptr_err = word; + ptr_err = trash.area; goto parse_err; } arg->data.sint = uint; @@ -253,10 +254,10 @@ int make_arg_list(const char *in, int len, uint64_t mask, struct arg **argp, break; case ARGT_SIZE: - if (in == beg) // empty size + if (!trash.data) // empty size goto empty_err; - ptr_err = parse_size_err(word, &uint); + ptr_err = parse_size_err(trash.area, &uint); if (ptr_err) goto parse_err; @@ -265,10 +266,10 @@ int make_arg_list(const char *in, int len, uint64_t mask, struct arg **argp, break; case ARGT_PBUF_FNUM: - if (in == beg) + if (!trash.data) goto empty_err; - if (!parse_dotted_uints(word, &arg->data.fid.ids, &arg->data.fid.sz)) + if (!parse_dotted_uints(trash.area, &arg->data.fid.ids, &arg->data.fid.sz)) goto parse_err; break; @@ -290,8 +291,6 @@ int make_arg_list(const char *in, int len, uint64_t mask, struct arg **argp, } end_parse: - free(word); word = NULL; - if (pos < min_arg) { /* not enough arguments */ memprintf(err_msg, @@ -307,11 +306,10 @@ int make_arg_list(const char *in, int len, uint64_t mask, struct arg **argp, in++; } else { /* the caller is responsible for freeing this message */ - word = (len > 0) ? my_strndup(in, len) : (char *)in; + char *word = (len > 0) ? my_strndup(in, len) : (char *)in; memprintf(err_msg, "expected ')' before '%s'", word); if (len > 0) free(word); - word = NULL; /* when we're missing a right paren, the empty part preceeding * already created an empty arg, adding one to the position, so * let's fix the reporting to avoid being confusing. @@ -331,7 +329,6 @@ int make_arg_list(const char *in, int len, uint64_t mask, struct arg **argp, return pos; err: - free(word); if (new_al == al) { /* only free the arg area if we have not queued unresolved args * still pointing to it. @@ -351,12 +348,18 @@ int make_arg_list(const char *in, int len, uint64_t mask, struct arg **argp, goto err; parse_err: + /* come here with the word attempted to parse in trash */ memprintf(err_msg, "failed to parse '%s' as type '%s' at position %d", - word, arg_type_names[(mask >> (pos * ARGT_BITS)) & ARGT_MASK], pos + 1); + trash.area, arg_type_names[(mask >> (pos * ARGT_BITS)) & ARGT_MASK], pos + 1); goto err; not_impl: memprintf(err_msg, "parsing for type '%s' was not implemented, please report this bug", arg_type_names[(mask >> (pos * ARGT_BITS)) & ARGT_MASK]); goto err; + + buffer_err: + memprintf(err_msg, "too small buffer size to store decoded argument %d, increase bufsize ?", + pos + 1); + goto err; }