Am 03.06.19 um 22:45 schrieb Matthew DeVore: > There is no reason to allow %00 to terminate a string, so do not allow it. > Otherwise, we end up returning arbitrary content in the string (that which is > after the %00) which is effectively hidden from callers and can escape sanity > checks and validation, and possible be used in tandem with a security > vulnerability to introduce a payload.
It's a bit hard to see with the (extended, but still) limited context, but url_decode_internal() effectively returns a NUL-terminated string, even though it does use a strbuf parameter named "out" for temporary storage. So callers really have no use for decoded NULs, and this change thus makes sense to me. > > Signed-off-by: Matthew DeVore <matv...@google.com> > --- > url.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/url.c b/url.c > index c0bb4e23c3..cf791cb139 100644 > --- a/url.c > +++ b/url.c > @@ -41,21 +41,21 @@ static char *url_decode_internal(const char **query, int > len, > if (!c) > break; > if (stop_at && strchr(stop_at, c)) { > q++; > len--; > break; > } > > if (c == '%' && len >= 3) { > int val = hex2chr(q + 1); > - if (0 <= val) { > + if (0 < val) { > strbuf_addch(out, val); > q += 3; > len -= 3; > continue; > } > } > > if (decode_plus && c == '+') > strbuf_addch(out, ' '); > else >