mirror of
http://git.haproxy.org/git/haproxy.git/
synced 2025-02-25 07:00:42 +00:00
BUG/MEDIUM: proxy: fix UAF with {tcp,http}checks logformat expressions
When parsing a logformat expression using parse_logformat_string(), the caller passes the proxy under which the expression is found as argument. This information allows the logformat expression API to check if the expression is compatible with the proxy settings. Since7a21c3a
("MAJOR: log: implement proper postparsing for logformat expressions"), the proxy compatibilty checks are postponed after the proxy is fully parsed to ensure proxy properties are fully resolved for checks consistency. The way it works, is that each time parse_logformat_string() is called for a given expression and proxy, it schedules the expression for postchecking by appending the expression to the list of pending expression checks on the proxy (lf_checks struct). Then, when the proxy is called with the REGISTER_POST_PROXY_CHECK() hook, it iterates over unchecked expressions and performs the check, then it removes the expression from its list. However, I overlooked a special case: if a logformat expression is used on a proxy that is disabled or a default proxy: REGISTER_POST_PROXY_CHECK() hook is never called. Because of that, lf expressions may still point to the proxy after the proxy is freed. For most logformat expressions, this isn't an issue because they are stored within the proxy itself, but this isn't the case with {tcp,http}checks logformat expressions: during deinit() sequence, all proxies are first cleaned up, and only then shared checks are freed. Because of that, the below config will trigger UAF since7a21c3a
: uaf.conf: listen dummy bind localhost:2222 backend testback disabled mode http option httpchk http-check send hdr test "test" http-check expect status 200 haproxy -f uaf.conf -c: ==152096== Invalid write of size 8 ==152096== at 0x21C317: lf_expr_deinit (log.c:3491) ==152096== by 0x2334A3: free_tcpcheck_http_hdr (tcpcheck.c:84) ==152096== by 0x2334A3: free_tcpcheck_http_hdr (tcpcheck.c:79) ==152096== by 0x2334A3: free_tcpcheck_http_hdrs (tcpcheck.c:98) ==152096== by 0x23365A: free_tcpcheck.part.0 (tcpcheck.c:130) ==152096== by 0x2338B1: free_tcpcheck (tcpcheck.c:108) ==152096== by 0x2338B1: deinit_tcpchecks (tcpcheck.c:3780) ==152096== by 0x2CF9A4: deinit (haproxy.c:2949) ==152096== by 0x2D0065: deinit_and_exit (haproxy.c:3052) ==152096== by 0x169BC0: main (haproxy.c:3996) ==152096== Address 0x52a8df8 is 6,968 bytes inside a block of size 7,168 free'd ==152096== at 0x484B27F: free (vg_replace_malloc.c:872) ==152096== by 0x2CF8AD: deinit (haproxy.c:2906) ==152096== by 0x2D0065: deinit_and_exit (haproxy.c:3052) ==152096== by 0x169BC0: main (haproxy.c:3996) To fix the issue, let's ensure in proxy_free_common() that no unchecked expressions may still point to the proxy after the proxy is freed by purging the list (DEL_INIT is used to reset list items). Special thanks to GH user @mhameed who filed a comprehensive issue with all the relevant information required to reproduce the bug (see GH #2597), after having first reported the issue on the alpine project bug tracker.
This commit is contained in:
parent
005e4ba715
commit
318c290ff2
@ -199,6 +199,7 @@ static inline void proxy_free_common(struct proxy *px)
|
||||
{
|
||||
struct acl *acl, *aclb;
|
||||
struct logger *log, *logb;
|
||||
struct lf_expr *lf, *lfb;
|
||||
|
||||
ha_free(&px->id);
|
||||
ha_free(&px->conf.file);
|
||||
@ -245,6 +246,13 @@ static inline void proxy_free_common(struct proxy *px)
|
||||
free_logger(log);
|
||||
}
|
||||
|
||||
/* ensure that remaining lf_expr that were not postchecked (ie: disabled
|
||||
* or default proxy) don't keep a reference on the proxy which is about
|
||||
* to be freed.
|
||||
*/
|
||||
list_for_each_entry_safe(lf, lfb, &px->conf.lf_checks, list)
|
||||
LIST_DEL_INIT(&lf->list);
|
||||
|
||||
chunk_destroy(&px->log_tag);
|
||||
|
||||
free_email_alert(px);
|
||||
|
Loading…
Reference in New Issue
Block a user