Hi,
below is a patch to fix a weakness by logging of broken header by incorrect proxy protocol. If some service (IDS/IPS) analyzing or monitoring log-file, regularly formatted lines may be simply confused with lines written not escaped directly from buffer supplied from foreign source. Not to mention it may open a certain vector allowing "injection" of user input in order to avoid detection of failures or even to simulate malicious traffic from legitimate service. How to reproduce: - enable proxy_protocol for listener and start nginx (here localhost on port 80); - echo 'set s [socket localhost 80]; puts $s "testntestntest"; close $s' | tclsh Error-log before fix: 2022/09/26 19:29:58 [error] 10104#17144: *3 broken header: "test test test " while reading PROXY protocol, client: 127.0.0.1, server: 0.0.0.0:80 Error-log after fix: 2022/09/26 22:48:50 [error] 13868#6132: *1 broken header: "test→→test→→test→→" while reading PROXY protocol, client: 127.0.0.1, server: 0.0.0.0:80 It is not advisable to log such foreign user input unescaped to the formatted log-file: instead of "...ntestn..." the attacker can write correctly formatted line simulating a 401-, 403-failure or rate-limit overran, so IDS could block a innocent service or mistakenly ban legitimate user. The patch proposes simplest escape (LF/CR-char with →, double quote with single quote and additionally every char larger or equal than 0x80 to avoid possible logging of "broken" utf-8 sequences or unsupported surrogates, just as a safest variant for not-valid foreign buffer) in-place in the malicious buffer directly (without mem-alloc, etc). Real life example - https://github.com/fail2ban/fail2ban/issues/3303#issuecomment-1148691902 Regards, Sergey. diff --git "a/src/core/ngx_proxy_protocol.c" "b/src/core/ngx_proxy_protocol.c" --- "a/src/core/ngx_proxy_protocol.c" +++ "b/src/core/ngx_proxy_protocol.c" @@ -139,6 +139,20 @@ skip: invalid: + p = buf; + while (p < last) { + const u_char c = *p; + switch (c) { + case LF: + case CR: + *p = (u_char)'x1a'; + break; + case '"': + *p = (u_char)'''; + break; + default: + if (c >= 0x80) {*p = '?';} + break; + } + p++; + } ngx_log_error(NGX_LOG_ERR, c->log, 0, "broken header: "%*s"", (size_t) (last - buf), buf);
_______________________________________________ nginx-devel mailing list -- nginx-devel@nginx.org To unsubscribe send an email to nginx-devel-le...@nginx.org
