BUG/MEDIUM: regex: fix risk of buffer overrun in exp_replace()

Currently exp_replace() (which is used in reqrep/reqirep) is
vulnerable to a buffer overrun. I have been able to reproduce it using
the attached configuration file and issuing the following command:

  wget -O - -S -q http://localhost:8000/`perl -e 'print "a"x4000'`/cookie.php

Str was being checked only in in while (str) and it was possible to
read past that when more than one character was being accessed in the
loop.

WT:
   Note that this bug is only marked MEDIUM because configurations
   capable of triggering this bug are very unlikely to exist at all due
   to the fact that most rewrites consist in static string additions
   that largely fit into the reserved area (8kB by default).

   This fix should also be backported to 1.4 and possibly even 1.3 since
   it seems to have been present since 1.1 or so.

Config:
-------

  global
        maxconn         500
        stats socket /tmp/haproxy.sock mode 600

  defaults
        timeout client      1000
        timeout connect      5000
        timeout server      5000
        retries         1
        option redispatch

  listen stats
        bind :8080
        mode http
        stats enable
        stats uri /stats
        stats show-legends

  listen  tcp_1
        bind :8000
        mode            http
        maxconn 400
        balance roundrobin
        reqrep ^([^\ :]*)\ /(.*)/(.*)\.php(.*) \1\ /\3.php?arg=\2\2\2\2\2\2\2\2\2\2\2\2\2\4
        server  srv1 127.0.0.1:9000 check port 9000 inter 1000 fall 1
        server  srv2 127.0.0.1:9001 check port 9001 inter 1000 fall 1

diff --git a/include/common/regex.h b/include/common/regex.h
index 9789ec3c9bea2eaa70c88b67e333f06eaf0a1bc1..63689afa36abd857ad5be1363e4594f3a7332916 100644 (file)
--- a/include/common/regex.h
+++ b/include/common/regex.h
@@ -79,7 +79,7 @@ extern regmatch_t pmatch[MAX_MATCH];
  * The function return 1 is succes case, else return 0 and err is filled.
  */
 int regex_comp(const char *str, struct my_regex *regex, int cs, int cap, char **err);
-int exp_replace(char *dst, char *src, const char *str, const regmatch_t *matches);
+int exp_replace(char *dst, uint dst_size, char *src, const char *str, const regmatch_t *matches);
 const char *check_replace_string(const char *str);
 const char *chain_regex(struct hdr_exp **head, const regex_t *preg,
                        int action, const char *replace, void *cond);

diff --git a/src/proto_http.c b/src/proto_http.c
index bd6d02425f0dab645f98cd1914a782b98d1e42fe..b833cfc01e3e624e8cca3d9bfe1eb23ed77c8bdb 100644 (file)
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -6817,7 +6817,10 @@ int apply_filter_to_req_headers(struct session *s, struct channel *req, struct h
                                break;
 
                        case ACT_REPLACE:
-                               trash.len = exp_replace(trash.str, cur_ptr, exp->replace, pmatch);
+                               trash.len = exp_replace(trash.str, trash.size, cur_ptr, exp->replace, pmatch);
+                               if (trash.len < 0)
+                                       return -1;
+
                                delta = buffer_replace2(req->buf, cur_ptr, cur_end, trash.str, trash.len);
                                /* FIXME: if the user adds a newline in the replacement, the
                                 * index will not be recalculated for now, and the new line
@@ -6927,7 +6930,10 @@ int apply_filter_to_req_line(struct session *s, struct channel *req, struct hdr_
 
                case ACT_REPLACE:
                        *cur_end = term; /* restore the string terminator */
-                       trash.len = exp_replace(trash.str, cur_ptr, exp->replace, pmatch);
+                       trash.len = exp_replace(trash.str, trash.size, cur_ptr, exp->replace, pmatch);
+                       if (trash.len < 0)
+                               return -1;
+
                        delta = buffer_replace2(req->buf, cur_ptr, cur_end, trash.str, trash.len);
                        /* FIXME: if the user adds a newline in the replacement, the
                         * index will not be recalculated for now, and the new line
@@ -7676,7 +7682,10 @@ int apply_filter_to_resp_headers(struct session *s, struct channel *rtr, struct
                                break;
 
                        case ACT_REPLACE:
-                               trash.len = exp_replace(trash.str, cur_ptr, exp->replace, pmatch);
+                               trash.len = exp_replace(trash.str, trash.size, cur_ptr, exp->replace, pmatch);
+                               if (trash.len < 0)
+                                       return -1;
+
                                delta = buffer_replace2(rtr->buf, cur_ptr, cur_end, trash.str, trash.len);
                                /* FIXME: if the user adds a newline in the replacement, the
                                 * index will not be recalculated for now, and the new line
@@ -7766,7 +7775,10 @@ int apply_filter_to_sts_line(struct session *s, struct channel *rtr, struct hdr_
 
                case ACT_REPLACE:
                        *cur_end = term; /* restore the string terminator */
-                       trash.len = exp_replace(trash.str, cur_ptr, exp->replace, pmatch);
+                       trash.len = exp_replace(trash.str, trash.size, cur_ptr, exp->replace, pmatch);
+                       if (trash.len < 0)
+                               return -1;
+
                        delta = buffer_replace2(rtr->buf, cur_ptr, cur_end, trash.str, trash.len);
                        /* FIXME: if the user adds a newline in the replacement, the
                         * index will not be recalculated for now, and the new line
@@ -7847,7 +7859,8 @@ int apply_filters_to_response(struct session *s, struct channel *rtr, struct pro
                        /* The filter did not match the response, it can be
                         * iterated through all headers.
                         */
-                       apply_filter_to_resp_headers(s, rtr, exp);
+                       if (unlikely(apply_filter_to_resp_headers(s, rtr, exp) < 0))
+                               return -1;
                }
        }
        return 0;

diff --git a/src/regex.c b/src/regex.c
index 7a7694050b41cd666f06c76bad1f3b89dc589ee9..b08147711542b531636a38bf6a47fde44a6be832 100644 (file)
--- a/src/regex.c
+++ b/src/regex.c
@@ -22,14 +22,17 @@
 /* regex trash buffer used by various regex tests */
 regmatch_t pmatch[MAX_MATCH];  /* rm_so, rm_eo for regular expressions */
 
-
-int exp_replace(char *dst, char *src, const char *str, const regmatch_t *matches)
+int exp_replace(char *dst, uint dst_size, char *src, const char *str, const regmatch_t *matches)
 {
        char *old_dst = dst;
+       char* dst_end = dst + dst_size;
 
        while (*str) {
                if (*str == '\\') {
                        str++;
+                       if (!*str)
+                               return -1;
+
                        if (isdigit((unsigned char)*str)) {
                                int len, num;
 
@@ -38,6 +41,10 @@ int exp_replace(char *dst, char *src, const char *str, const regmatch_t *matches
 
                                if (matches[num].rm_eo > -1 && matches[num].rm_so > -1) {
                                        len = matches[num].rm_eo - matches[num].rm_so;
+
+                                       if (dst + len >= dst_end)
+                                               return -1;
+
                                        memcpy(dst, src + matches[num].rm_so, len);
                                        dst += len;
                                }
@@ -46,19 +53,39 @@ int exp_replace(char *dst, char *src, const char *str, const regmatch_t *matches
                                unsigned char hex1, hex2;
                                str++;
 
+                               if (!*str)
+                                       return -1;
+
                                hex1 = toupper(*str++) - '0';
+
+                               if (!*str)
+                                       return -1;
+
                                hex2 = toupper(*str++) - '0';
 
                                if (hex1 > 9) hex1 -= 'A' - '9' - 1;
                                if (hex2 > 9) hex2 -= 'A' - '9' - 1;
+
+                               if (dst >= dst_end)
+                                       return -1;
+
                                *dst++ = (hex1<<4) + hex2;
                        } else {
+                               if (dst >= dst_end)
+                                       return -1;
+
                                *dst++ = *str++;
                        }
                } else {
+                       if (dst >= dst_end)
+                               return -1;
+
                        *dst++ = *str++;
                }
        }
+       if (dst >= dst_end)
+               return -1;
+
        *dst = '\0';
        return dst - old_dst;
 }