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
This commit is contained in:
Sasha Pachev 2014-05-26 12:33:48 -06:00 committed by Willy Tarreau
parent 2705a61d8c
commit c600204ddf
3 changed files with 48 additions and 8 deletions

View File

@ -79,7 +79,7 @@ extern regmatch_t pmatch[MAX_MATCH];
* The function return 1 is succes case, else return 0 and err is filled. * 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 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 *check_replace_string(const char *str);
const char *chain_regex(struct hdr_exp **head, const regex_t *preg, const char *chain_regex(struct hdr_exp **head, const regex_t *preg,
int action, const char *replace, void *cond); int action, const char *replace, void *cond);

View File

@ -6817,7 +6817,10 @@ int apply_filter_to_req_headers(struct session *s, struct channel *req, struct h
break; break;
case ACT_REPLACE: 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); delta = buffer_replace2(req->buf, cur_ptr, cur_end, trash.str, trash.len);
/* FIXME: if the user adds a newline in the replacement, the /* FIXME: if the user adds a newline in the replacement, the
* index will not be recalculated for now, and the new line * 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: case ACT_REPLACE:
*cur_end = term; /* restore the string terminator */ *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); delta = buffer_replace2(req->buf, cur_ptr, cur_end, trash.str, trash.len);
/* FIXME: if the user adds a newline in the replacement, the /* FIXME: if the user adds a newline in the replacement, the
* index will not be recalculated for now, and the new line * 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; break;
case ACT_REPLACE: 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); delta = buffer_replace2(rtr->buf, cur_ptr, cur_end, trash.str, trash.len);
/* FIXME: if the user adds a newline in the replacement, the /* FIXME: if the user adds a newline in the replacement, the
* index will not be recalculated for now, and the new line * 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: case ACT_REPLACE:
*cur_end = term; /* restore the string terminator */ *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); delta = buffer_replace2(rtr->buf, cur_ptr, cur_end, trash.str, trash.len);
/* FIXME: if the user adds a newline in the replacement, the /* FIXME: if the user adds a newline in the replacement, the
* index will not be recalculated for now, and the new line * 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 /* The filter did not match the response, it can be
* iterated through all headers. * 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; return 0;

View File

@ -22,14 +22,17 @@
/* regex trash buffer used by various regex tests */ /* regex trash buffer used by various regex tests */
regmatch_t pmatch[MAX_MATCH]; /* rm_so, rm_eo for regular expressions */ regmatch_t pmatch[MAX_MATCH]; /* rm_so, rm_eo for regular expressions */
int exp_replace(char *dst, uint dst_size, char *src, const char *str, const regmatch_t *matches)
int exp_replace(char *dst, char *src, const char *str, const regmatch_t *matches)
{ {
char *old_dst = dst; char *old_dst = dst;
char* dst_end = dst + dst_size;
while (*str) { while (*str) {
if (*str == '\\') { if (*str == '\\') {
str++; str++;
if (!*str)
return -1;
if (isdigit((unsigned char)*str)) { if (isdigit((unsigned char)*str)) {
int len, num; 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) { if (matches[num].rm_eo > -1 && matches[num].rm_so > -1) {
len = matches[num].rm_eo - matches[num].rm_so; len = matches[num].rm_eo - matches[num].rm_so;
if (dst + len >= dst_end)
return -1;
memcpy(dst, src + matches[num].rm_so, len); memcpy(dst, src + matches[num].rm_so, len);
dst += 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; unsigned char hex1, hex2;
str++; str++;
if (!*str)
return -1;
hex1 = toupper(*str++) - '0'; hex1 = toupper(*str++) - '0';
if (!*str)
return -1;
hex2 = toupper(*str++) - '0'; hex2 = toupper(*str++) - '0';
if (hex1 > 9) hex1 -= 'A' - '9' - 1; if (hex1 > 9) hex1 -= 'A' - '9' - 1;
if (hex2 > 9) hex2 -= 'A' - '9' - 1; if (hex2 > 9) hex2 -= 'A' - '9' - 1;
if (dst >= dst_end)
return -1;
*dst++ = (hex1<<4) + hex2; *dst++ = (hex1<<4) + hex2;
} else { } else {
if (dst >= dst_end)
return -1;
*dst++ = *str++; *dst++ = *str++;
} }
} else { } else {
if (dst >= dst_end)
return -1;
*dst++ = *str++; *dst++ = *str++;
} }
} }
if (dst >= dst_end)
return -1;
*dst = '\0'; *dst = '\0';
return dst - old_dst; return dst - old_dst;
} }