mod_proxy: tighten interim response handling

modules/http/http_filters.c

@@ -978,7 +978,7 @@ static apr_status_t validate_status_line(request_rec *r)
         if (len < 3
             || apr_strtoi64(r->status_line, &end, 10) != r->status
             || (end - 3) != r->status_line
-            || (len >= 4 && ! apr_isspace(r->status_line[3]))) {
+            || (len >= 4 && r->status_line[3] != ' ')) {
             r->status_line = NULL;
             return APR_EGENERAL;
         }

server/protocol.c

@@ -2321,6 +2321,11 @@ AP_DECLARE(void) ap_send_interim_response(request_rec *r, int send_headers)
     }
     if (r->status_line && strlen(r->status_line) > 4) {
         reason = r->status_line + 4;
+        /* Reject control characters in the reason phrase to prevent
+         * forwarding backend-controlled bare CR in the status line. */
+        if (*ap_scan_http_field_content(reason)) {
+            reason = NULL;
+        }
     }
     b = ap_bucket_response_create(r->status, reason,
                                   send_headers? r->headers_out : NULL,

test/modules/proxy/test_06_interim_resp.py

@@ -0,0 +1,182 @@
+import socket
+from threading import Thread
+
+import pytest
+
+from pyhttpd.conf import HttpdConf
+from .env import TCPFaker
+
+
+class _StatusLineBackend(TCPFaker):
+    """Backend that sends various status line formats."""
+
+    def __init__(self, host, port, mode="final-status-sep-cr"):
+        super().__init__(host, port)
+        self._mode = mode
+
+    def _make_response(self, data):
+        if self._mode == "final-status-sep-cr":
+            return (
+                b"HTTP/1.1 200\rX-Foobar: abc\r\n"
+                b"Content-Length: 2\r\n"
+                b"Content-Type: text/plain\r\n"
+                b"\r\n"
+                b"OK"
+            )
+        elif self._mode == "interim-status-cr":
+            return (
+                b"HTTP/1.1 103 Early\rX-Foobar: abc\r\n"
+                b"X-Early: whatever\r\n"
+                b"\r\n"
+                b"HTTP/1.1 200 OK\r\n"
+                b"Content-Length: 2\r\n"
+                b"Content-Type: text/plain\r\n"
+                b"\r\n"
+                b"OK"
+            )
+        elif self._mode == "interim-102":
+            return (
+                b"HTTP/1.1 102 Processing\r\n"
+                b"\r\n"
+                b"HTTP/1.1 200 OK\r\n"
+                b"Content-Length: 2\r\n"
+                b"Content-Type: text/plain\r\n"
+                b"\r\n"
+                b"OK"
+            )
+        return super()._make_response(data)
+
+
+def _recv_all(sock, timeout=5):
+    sock.settimeout(timeout)
+    data = b""
+    while True:
+        try:
+            chunk = sock.recv(4096)
+            if not chunk:
+                break
+            data += chunk
+        except socket.timeout:
+            break
+    return data
+
+
+class TestStatusLineCR:
+    """Verify that bare CR in backend status lines is rejected.
+
+    Two cases:
+    1. Final response with CR at the separator position (byte 3 of status_line)
+    2. Interim 1xx response with CR in the reason phrase
+    """
+
+    @pytest.fixture(autouse=True, scope='class')
+    def _class_scope(self, env):
+        conf = HttpdConf(env)
+        conf.start_vhost(domains=[f"test1.{env.http_tld}"], port=env.http_port,
+                         doc_root="htdocs", with_ssl=False)
+        conf.add([
+            f"ProxyPass / http://127.0.0.1:{env.http_port2}/",
+            f"ProxyPassReverse / http://127.0.0.1:{env.http_port2}/",
+        ])
+        conf.end_vhost()
+        conf.install()
+        assert env.apache_restart() == 0
+        yield
+
+    def test_proxy_06_001_final_status_sep_cr(self, env):
+        """A final response with CR at status-code separator must not
+        forward attacker text as the reason phrase."""
+        faker = _StatusLineBackend("127.0.0.1", env.http_port2,
+                                 mode="final-status-sep-cr")
+        faker.start()
+        try:
+            with socket.create_connection(('localhost', int(env.http_port))) as sock:
+                req = (
+                    f"GET / HTTP/1.0\r\n"
+                    f"Host: test1.{env.http_tld}\r\n"
+                    f"\r\n"
+                )
+                sock.sendall(req.encode())
+                sock.shutdown(socket.SHUT_WR)
+                raw = _recv_all(sock)
+        finally:
+            faker.stop()
+
+        status_line = raw.split(b"\r\n")[0]
+        assert b"\r" not in status_line[:-1] if status_line.endswith(b"\r") else b"\r" not in status_line, \
+            f"bare CR in status line: {status_line!r}"
+        assert b"X-Foobar" not in status_line, \
+            f"attacker text in status line: {status_line!r}"
+        assert b"X-Foobar" not in raw.split(b"\r\n\r\n")[0], \
+            f"injected header in response headers: {raw.split(b'\\r\\n\\r\\n')[0]!r}"
+
+        env.httpd_error_log.ignore_recent(
+            lognos=["AH00957", "AH01106", "AH01114"]
+        )
+
+    def test_proxy_06_002_interim_status_cr(self, env):
+        """An interim 103 response with CR in the reason phrase must not
+        forward the bare CR to the client."""
+        faker = _StatusLineBackend("127.0.0.1", env.http_port2,
+                                 mode="interim-status-cr")
+        faker.start()
+        try:
+            with socket.create_connection(('localhost', int(env.http_port))) as sock:
+                req = (
+                    f"GET / HTTP/1.1\r\n"
+                    f"Host: test1.{env.http_tld}\r\n"
+                    f"Connection: close\r\n"
+                    f"\r\n"
+                )
+                sock.sendall(req.encode())
+                sock.shutdown(socket.SHUT_WR)
+                raw = _recv_all(sock)
+        finally:
+            faker.stop()
+
+        # Split into individual response blocks. The 103 interim response
+        # comes before the final 200.  Check every status line for bare CR.
+        lines = raw.split(b"\r\n")
+        for line in lines:
+            if line.startswith(b"HTTP/"):
+                assert b"\r" not in line, \
+                    f"bare CR in status line: {line!r}"
+                assert b"X-Foobar" not in line, \
+                    f"attacker text in status line: {line!r}"
+
+        headers_section = raw.split(b"\r\n\r\n")[0]
+        assert b"X-Foobar" not in headers_section, \
+            f"injected header in response: {headers_section!r}"
+
+        env.httpd_error_log.ignore_recent(
+            lognos=["AH01106"]
+        )
+
+    def test_proxy_06_003_interim_102_ok(self, env):
+        """A well-formed 102 Processing interim response is forwarded
+        correctly, followed by the final 200."""
+        faker = _StatusLineBackend("127.0.0.1", env.http_port2,
+                                   mode="interim-102")
+        faker.start()
+        try:
+            with socket.create_connection(('localhost', int(env.http_port))) as sock:
+                req = (
+                    f"GET / HTTP/1.1\r\n"
+                    f"Host: test1.{env.http_tld}\r\n"
+                    f"Connection: close\r\n"
+                    f"\r\n"
+                )
+                sock.sendall(req.encode())
+                sock.shutdown(socket.SHUT_WR)
+                raw = _recv_all(sock)
+        finally:
+            faker.stop()
+
+        status_lines = [l for l in raw.split(b"\r\n")
+                        if l.startswith(b"HTTP/")]
+        assert len(status_lines) == 2, \
+            f"expected 2 status lines (102 + 200), got {len(status_lines)}: {status_lines!r}"
+        assert b"102" in status_lines[0], \
+            f"first status line should be 102: {status_lines[0]!r}"
+        assert b"200" in status_lines[1], \
+            f"second status line should be 200: {status_lines[1]!r}"