<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN">
<html><body style='font-size: 10pt; font-family: Verdana,Geneva,sans-serif'>
<p>Hi,</p>
<p>below is a patch to fix a weakness by logging of broken header by incorrect proxy protocol.</p>
<p>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.<br />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.</p>
<p>How to reproduce:</p>
<p>- enable proxy_protocol for listener and start nginx (here localhost on port 80);<br />- <span style="font-family: courier new,courier;">echo 'set s [socket localhost 80]; puts $s "test\ntest\ntest"; close $s' | tclsh</span><br /><br />Error-log before fix:</p>
<p><span style="font-family: courier new,courier;">2022/09/26 19:29:58 [error] 10104#17144: *3 broken header: "test</span><br /><span style="font-family: courier new,courier;">test</span><br /><span style="font-family: courier new,courier;">test</span><br /><span style="font-family: courier new,courier;">" while reading PROXY protocol, client: 127.0.0.1, server: 0.0.0.0:80</span></p>
<p>Error-log after fix:</p>
<p><span style="font-family: courier new,courier;">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</span></p>
<p>It is not advisable to log such foreign user input unescaped to the formatted log-file: instead of <span style="font-family: courier new,courier;">"...\ntest\n..."</span> 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.</p>
<p>The patch proposes simplest escape (LF/CR-char with <span style="font-family: courier new,courier;">→</span>, 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).</p>
<p>Real life example - https://github.com/fail2ban/fail2ban/issues/3303#issuecomment-1148691902</p>
<p>Regards,<br />Sergey.</p>
<p><span style="font-family: courier new,courier;">diff --git "a/src/core/ngx_proxy_protocol.c" "b/src/core/ngx_proxy_protocol.c"</span><br /><span style="font-family: courier new,courier;">--- "a/src/core/ngx_proxy_protocol.c"</span><br /><span style="font-family: courier new,courier;">+++ "b/src/core/ngx_proxy_protocol.c"</span><br /><span style="font-family: courier new,courier;">@@ -139,6 +139,20 @@ skip:</span><br /><span style="font-family: courier new,courier;"> </span><br /><span style="font-family: courier new,courier;"> invalid:</span><br /><span style="font-family: courier new,courier;"> </span><br /><span style="font-family: courier new,courier;">+ p = buf;</span><br /><span style="font-family: courier new,courier;">+ while (p < last) {</span><br /><span style="font-family: courier new,courier;">+ const u_char c = *p;</span><br /><span style="font-family: courier new,courier;">+ switch (c) {</span><br /><span style="font-family: courier new,courier;">+ case LF:</span><br /><span style="font-family: courier new,courier;">+ case CR:</span><br /><span style="font-family: courier new,courier;">+ *p = (u_char)'\x1a';</span><br /><span style="font-family: courier new,courier;">+ break;</span><br /><span style="font-family: courier new,courier;"><span style="font-family: courier new,courier;">+ case '"':</span><br /><span style="font-family: courier new,courier;">+ *p = (u_char)'\'';</span><br /><span style="font-family: courier new,courier;">+ break;</span><br /><span style="font-family: courier new,courier;"></span>+ default:</span><br /><span style="font-family: courier new,courier;">+ if (c >= 0x80) {*p = '?';}</span><br /><span style="font-family: courier new,courier;">+ break;</span><br /><span style="font-family: courier new,courier;">+ }</span><br /><span style="font-family: courier new,courier;">+ p++;</span><br /><span style="font-family: courier new,courier;">+ }</span><br /><span style="font-family: courier new,courier;"> ngx_log_error(NGX_LOG_ERR, c->log, 0,</span><br /><span style="font-family: courier new,courier;"> "broken header: \"%*s\"", (size_t) (last - buf), buf);</span><br /><span style="font-family: courier new,courier;"> </span><br /><br /></p>
</body></html>