On Fri, Jan 5, 2024 at 3:11 AM Eric Covener <cove...@gmail.com> wrote: > > On Thu, Jan 4, 2024 at 9:04 PM Jason Pyeron <jpye...@pdinc.us> wrote: > > > > I am having some issue searching Bugzilla for any issue involving > > process_regexp in mod_headers.c . > > > > It finds nothing, so I am assuming I did something wrong in my search. Will > > file bug if not already filed. > > > > We are investigating an infinite loop (stack overflow) issue, caused by > > "securing" a system. > > > > ZZZ-STIG-SV-214288r881493_rule.conf:Header always edit* Set-Cookie ^(.*)$ > > $1;HttpOnly;secure > > edit* just makes no sense at all here when you are capturing/replacing > the entire string and will loop by definition.
Maybe we should avoid infinite recursion in any case, like in the attached patch? How does it work for you Jason? Regards; Yann.
Index: modules/metadata/mod_headers.c =================================================================== --- modules/metadata/mod_headers.c (revision 1914804) +++ modules/metadata/mod_headers.c (working copy) @@ -622,41 +622,70 @@ static char* process_tags(header_entry *hdr, reque } return str ? str : ""; } -static const char *process_regexp(header_entry *hdr, const char *value, - request_rec *r) + +static const char *process_regexp_rec(header_entry *hdr, const char *value, + request_rec *r, ap_regmatch_t pmatch[], + int flags) { - ap_regmatch_t pmatch[AP_MAX_REG_MATCH]; - const char *subs; - const char *remainder; char *ret; - int diffsz; - if (ap_regexec(hdr->regex, value, AP_MAX_REG_MATCH, pmatch, 0)) { + const char *subs, *remainder; + apr_size_t val_len, subs_len, rem_len; + apr_size_t match_start, match_end; + + val_len = strlen(value); + if (ap_regexec_len(hdr->regex, value, val_len, + AP_MAX_REG_MATCH, pmatch, + flags)) { /* no match, nothing to do */ return value; } + /* Process tags in the input string rather than the resulting * substitution to avoid surprises */ - subs = ap_pregsub(r->pool, process_tags(hdr, r), value, AP_MAX_REG_MATCH, pmatch); + subs = ap_pregsub(r->pool, process_tags(hdr, r), value, + AP_MAX_REG_MATCH, pmatch); if (subs == NULL) return NULL; - diffsz = strlen(subs) - (pmatch[0].rm_eo - pmatch[0].rm_so); + subs_len = strlen(subs); + + ap_assert(pmatch[0].rm_so >= 0 && pmatch[0].rm_so <= pmatch[0].rm_eo); + match_start = pmatch[0].rm_so; + match_end = pmatch[0].rm_eo; if (hdr->action == hdr_edit) { - remainder = value + pmatch[0].rm_eo; + remainder = value + match_end; } else { /* recurse to edit multiple matches if applicable */ - remainder = process_regexp(hdr, value + pmatch[0].rm_eo, r); - if (remainder == NULL) - return NULL; - diffsz += strlen(remainder) - strlen(value + pmatch[0].rm_eo); + if (match_end == 0 && val_len) { + /* advance on empty match to avoid infinite recursion */ + match_start = match_end = 1; + } + if (match_end < val_len) { + remainder = process_regexp_rec(hdr, value + match_end, + r, pmatch, AP_REG_NOTBOL); + if (remainder == NULL) + return NULL; + } + else { + remainder = ""; + } } - ret = apr_palloc(r->pool, strlen(value) + 1 + diffsz); - memcpy(ret, value, pmatch[0].rm_so); - strcpy(ret + pmatch[0].rm_so, subs); - strcat(ret, remainder); + rem_len = strlen(remainder); + + ret = apr_palloc(r->pool, match_start + subs_len + rem_len + 1); + memcpy(ret, value, match_start); + memcpy(ret + match_start, subs, subs_len); + memcpy(ret + match_start + subs_len, remainder, rem_len + 1); return ret; } +static const char *process_regexp(header_entry *hdr, const char *value, + request_rec *r) +{ + ap_regmatch_t pmatch[AP_MAX_REG_MATCH]; + return process_regexp_rec(hdr, value, r, pmatch, 0); +} + static int echo_header(void *v, const char *key, const char *val) { echo_do *ed = (echo_do *)v;