The calls to strbuf_add* within append_normalized_escapes() can
reallocate the buffer passed to it.  Therefore, the seg_start pointer
into the string cannot be kept across such calls.

The actual bug is from 3402a8d (config: add helper to normalize and
match URLs, 2013-07-31).  It can first be detected by valgrind after
6a56993 (config: parse http.<url>.<variable> using urlmatch,
2013-08-05) introduced tests covering url_normalize().

Signed-off-by: Thomas Rast <tr...@inf.ethz.ch>
---

My apologies if this is redundant; I didn't have time to watch the
list over the last two weeks.  However it seems today's pu is still
broken.

The valgrind error looks like this:

  ==4607== Invalid read of size 1
  ==4607==    at 0x4C2D3A1: __GI_strcmp (mc_replace_strmem.c:731)
  ==4607==    by 0x404C68: url_normalize (urlmatch.c:300)
  ==4607==    by 0x403F33: main (test-urlmatch-normalization.c:34)
  ==4607==  Address 0x5be9046 is 6 bytes inside a block of size 24 free'd
  ==4607==    at 0x4C2BFC6: realloc (vg_replace_malloc.c:687)
  ==4607==    by 0x405F6B: xrealloc (wrapper.c:100)
  ==4607==    by 0x40794E: strbuf_grow (strbuf.c:74)
  ==4607==    by 0x40854D: strbuf_vaddf (strbuf.c:268)
  ==4607==    by 0x40817E: strbuf_addf (strbuf.c:203)
  ==4607==    by 0x404300: append_normalized_escapes (urlmatch.c:58)
  ==4607==    by 0x404C0A: url_normalize (urlmatch.c:291)
  ==4607==    by 0x403F33: main (test-urlmatch-normalization.c:34)

It went undetected for a while because it does not fail the test: the
calls to test-urlmatch-normalization happen inside a $() substitution.

I checked the other call sites to append_normalized_escapes() for the
same type of problem, and they seem to be okay.

 urlmatch.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/urlmatch.c b/urlmatch.c
index 1db76c8..59abc80 100644
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -281,7 +281,8 @@ char *url_normalize(const char *url, struct url_info 
*out_info)
                url_len--;
        }
        for (;;) {
-               const char *seg_start = norm.buf + norm.len;
+               const char *seg_start;
+               size_t prev_len = norm.len;
                const char *next_slash = url + strcspn(url, "/?#");
                int skip_add_slash = 0;
                /*
@@ -297,6 +298,7 @@ char *url_normalize(const char *url, struct url_info 
*out_info)
                        strbuf_release(&norm);
                        return NULL;
                }
+               seg_start = norm.buf + prev_len;
                if (!strcmp(seg_start, ".")) {
                        /* ignore a . segment; be careful not to remove initial 
'/' */
                        if (seg_start == path_start + 1) {
-- 
1.8.4.609.g4395a4f

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to