Matthew DeVore <matvore@xxxxxxxxxx> writes: > url_decode_internal could have been tricked into reading past the length > of the **query buffer if there are fewer than 2 characters after a % (in > a null-terminated string, % would have to be the last character). > Prevent this from happening by checking len before decoding the % > sequence. > > Helped-by: René Scharfe <l.s.r@xxxxxx> > Signed-off-by: Matthew DeVore <matvore@xxxxxxxxxx> > --- > url.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/url.c b/url.c > index 25576c390b..9ea9d5611b 100644 > --- a/url.c > +++ b/url.c > @@ -39,21 +39,21 @@ static char *url_decode_internal(const char **query, int len, > unsigned char c = *q; > > if (!c) > break; > if (stop_at && strchr(stop_at, c)) { > q++; > len--; > break; > } > > - if (c == '%') { > + if (c == '%' && (len < 0 || len >= 3)) { > int val = hex2chr(q + 1); This made me wonder what happens when the caller sent -1 in len, but hex2chr() stops on such a string with % plus one hexadecimal at the end of the string, and we'd end up copying these two bytes one at a time, which is what we want, so it is OK. And the rejection of %00 done in 2/2 follows the same codeflow here, which is quite straight forward. Nice. > if (0 <= val) { > strbuf_addch(out, val); > q += 3; > len -= 3; > continue; > } > } > > if (decode_plus && c == '+')