fix: reject request headers that contain CR or LF

README.md

@@ -90,6 +90,8 @@ When any of the three request functions (`http_get`, `http_post`, `http_delete`)
 
 Once a response is received, it gets stored in the `_http_response` table. By monitoring this table, you can keep track of response statuses and messages.
 
+A request whose headers contain a carriage return or line feed is not sent. It gets an `ERROR` response naming the header, since libcurl terminates headers with CRLF and an embedded one would let the header smuggle extra headers or a body into the request.
+
 > [!IMPORTANT]
 > Inserting directly into `net.http_request_queue` won't wake the worker, you must use the request functions. Rows inserted directly are only processed the next time the worker wakes up.
 > We do it this way to avoid polling the `net.http_request_queue` table, which would pollute `pg_stat_statements` and cause unnecessary activity from the worker.

src/core.c

@@ -22,7 +22,21 @@ static size_t body_cb(void *contents, size_t size, size_t nmemb, void *userp) {
   return realsize;
 }
 
-static struct curl_slist *pg_text_array_to_slist(ArrayType *array, struct curl_slist *headers) {
+// A header with a CR or LF in it can inject extra headers or a body into the request, since libcurl
+// ends every header with CRLF. Such requests are not sent. The message only names the header, the
+// value may hold credentials.
+static char *crlf_header_rejection(const char *hdr) {
+  size_t bad = strcspn(hdr, "\r\n");
+  if (hdr[bad] == '\0') return NULL;
+
+  size_t name_len = strcspn(hdr, ":");
+  if (name_len > bad) name_len = bad;
+
+  return psprintf("header \"%.*s\" contains a carriage return or line feed", (int)name_len, hdr);
+}
+
+static struct curl_slist *pg_text_array_to_slist(ArrayType *array, struct curl_slist *headers,
+                                                 char **rejected_reason) {
   ArrayIterator iterator;
   Datum         value;
   bool          isnull;
@@ -36,7 +50,17 @@ static struct curl_slist *pg_text_array_to_slist(ArrayType *array, struct curl_s
     }
 
     hdr = TextDatumGetCString(value);
-    EREPORT_CURL_SLIST_APPEND(headers, hdr);
+
+    char *reason = crlf_header_rejection(hdr);
+    if (reason) {
+      if (*rejected_reason == NULL)
+        *rejected_reason = reason;
+      else
+        pfree(reason);
+    } else {
+      EREPORT_CURL_SLIST_APPEND(headers, hdr);
+    }
+
     pfree(hdr);
   }
   array_free_iterator(iterator);
@@ -67,7 +91,7 @@ void init_curl_handle(CurlHandle *handle, RequestQueueRow row) {
     ArrayType         *pgHeaders       = DatumGetArrayTypeP(row.headersBin.value);
     struct curl_slist *request_headers = NULL;
 
-    request_headers = pg_text_array_to_slist(pgHeaders, request_headers);
+    request_headers = pg_text_array_to_slist(pgHeaders, request_headers, &handle->rejected_reason);
 
     EREPORT_CURL_SLIST_APPEND(request_headers, "User-Agent: pg_net/" EXTVERSION);
 

test/test_http_header_crlf.py

@@ -0,0 +1,120 @@
+import pytest
+from common import pg_collect_response, pg_http_request
+from test_http_timeout import wait_for_responses
+
+
+@pytest.mark.parametrize(
+    "header, reported_name",
+    [
+        ('{"X-Test": "value\\n"}', "X-Test"),
+        ('{"X-Test": "value\\r"}', "X-Test"),
+        ('{"X-Test": "value\\r\\nInjected: yes"}', "X-Test"),
+        ('{"X-Te\\nst": "value"}', "X-Te"),
+        ('{"\\r\\nInjected": "yes"}', ""),
+    ],
+)
+def test_headers_with_cr_or_lf_are_rejected(conn, header, reported_name):
+    """A header containing CR or LF is not sent and gets an ERROR response naming the header"""
+
+    request_id = pg_http_request(
+        conn,
+        "select net.http_get(url := 'http://localhost:8080/headers', headers := %s::jsonb)",
+        (header,),
+    )
+
+    (status_code, error_msg, timed_out) = wait_for_responses(conn, [request_id])[
+        request_id
+    ]
+
+    assert status_code is None
+    assert timed_out is False
+    assert (
+        error_msg == f'header "{reported_name}" contains a carriage return or line feed'
+    )
+    assert "value" not in error_msg
+    assert "Injected" not in error_msg
+
+
+def test_headers_without_cr_or_lf_are_sent(conn):
+    """Ordinary headers still reach the server"""
+
+    request_id = pg_http_request(
+        conn,
+        """select net.http_get(
+            url := 'http://localhost:8080/headers',
+            headers := '{"X-Test": "plain value", "accept": "application/json"}'::jsonb
+        )""",
+    )
+
+    response = pg_collect_response(conn, request_id)
+
+    assert response["status"] == "SUCCESS"
+    assert "X-Test" in response["body"]
+
+
+def test_rejected_header_does_not_affect_the_rest_of_the_batch(conn):
+    """One bad header in a batch gets an ERROR row, the other requests are sent normally"""
+
+    bad = pg_http_request(
+        conn,
+        """select net.http_get(url := 'http://localhost:8080/headers', headers := '{"X-Test": "a\\nb"}'::jsonb)""",
+    )
+    get = pg_http_request(
+        conn, "select net.http_get(url := 'http://localhost:8080/headers')"
+    )
+    post = pg_http_request(
+        conn,
+        "select net.http_post(url := 'http://localhost:8080/anything', body := '{}'::jsonb)",
+    )
+
+    responses = wait_for_responses(conn, [bad, get, post])
+
+    assert responses[bad][0] is None
+    assert (
+        responses[bad][1] == 'header "X-Test" contains a carriage return or line feed'
+    )
+    assert responses[get][0] == 200
+    assert responses[post][0] == 200
+
+
+def test_direct_insert_with_cr_or_lf_header_is_rejected(conn):
+    """Rows inserted straight into the queue go through the same check"""
+
+    request_id = pg_http_request(
+        conn,
+        """insert into net.http_request_queue(method, url, headers, timeout_milliseconds)
+           values ('GET', 'http://localhost:8080/headers', '{"X-Test": "a\\rb"}'::jsonb, 5000)
+           returning id""",
+    )
+    conn.execute("select net.wake()")
+    conn.commit()
+
+    (status_code, error_msg, _) = wait_for_responses(conn, [request_id])[request_id]
+
+    assert status_code is None
+    assert error_msg == 'header "X-Test" contains a carriage return or line feed'
+
+    follow_up = pg_http_request(
+        conn, "select net.http_get(url := 'http://localhost:8080/headers')"
+    )
+    assert wait_for_responses(conn, [follow_up])[follow_up][0] == 200
+
+
+def test_timeout_rejection_is_reported_before_the_header_one(conn):
+    """A request with both a bad timeout and a bad header gets the timeout message"""
+
+    request_id = pg_http_request(
+        conn,
+        """select net.http_get(
+            url := 'http://localhost:8080/headers',
+            headers := '{"X-Test": "a\\nb"}'::jsonb,
+            timeout_milliseconds := 0
+        )""",
+    )
+
+    (_, error_msg, _) = wait_for_responses(conn, [request_id])[request_id]
+
+    assert (
+        error_msg
+        == "timeout_milliseconds must be between 1 and 600000 (pg_net.max_timeout_ms), got 0"
+    )