This patch fixes a theoretically possible segfault in mod_proxy_http2.
Please see the open_stream() function in h2_proxy_session.c:598. If the
call to apr_uri_parse() fails, some of the apr_uri_t's fields may be set
to NULL, and this would cause a segfault in the following lines:
authority = puri.hostname;
if (!ap_strchr_c(authority, ':') && puri.port
...
Currently, the URIs are preprocessed by ap_proxy_canon_netloc() much earlier
than opening the proxy stream, but this may not hold in the future. As well
as that, I think that there might be subtle differences between how these two
functions handle invalid URIs, and that could also result in the crash.
I attached the patch with a fix for this issue.
While here, I also spotted an opportunity for a minor code cleanup. There
are a couple of places where a function returns an apr_status_t, but the
calling site tests the result against the OK (hook return code). I updated
such places in the second patch.
Regards,
Evgeny Kotkov
mod_proxy_http2: Guard against theoretically possible segfault with
invalid URIs.
Index: modules/http2/h2_proxy_session.c
===================================================================
--- modules/http2/h2_proxy_session.c (revision 1743495)
+++ modules/http2/h2_proxy_session.c (working copy)
@@ -581,6 +581,7 @@ static apr_status_t open_stream(h2_proxy_session *
h2_proxy_stream *stream;
apr_uri_t puri;
const char *authority, *scheme, *path;
+ apr_status_t status;
stream = apr_pcalloc(r->pool, sizeof(*stream));
@@ -595,7 +596,10 @@ static apr_status_t open_stream(h2_proxy_session *
stream->req = h2_req_create(1, stream->pool, 0);
- apr_uri_parse(stream->pool, url, &puri);
+ status = apr_uri_parse(stream->pool, url, &puri);
+ if (status != APR_SUCCESS)
+ return status;
+
scheme = (strcmp(puri.scheme, "h2")? "http" : "https");
authority = puri.hostname;
if (!ap_strchr_c(authority, ':') && puri.port
mod_proxy_http2: Fix minor inconsistency when checking apr_status_t values.
Index: modules/http2/h2_proxy_session.c
===================================================================
--- modules/http2/h2_proxy_session.c (revision 1743495)
+++ modules/http2/h2_proxy_session.c (working copy)
@@ -763,7 +763,7 @@ apr_status_t h2_proxy_session_submit(h2_proxy_sess
apr_status_t status;
status = open_stream(session, url, r, &stream);
- if (status == OK) {
+ if (status == APR_SUCCESS) {
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(03381)
"process stream(%d): %s %s%s, original: %s",
stream->id, stream->req->method,
Index: modules/http2/mod_proxy_http2.c
===================================================================
--- modules/http2/mod_proxy_http2.c (revision 1743495)
+++ modules/http2/mod_proxy_http2.c (working copy)
@@ -259,7 +259,7 @@ static apr_status_t add_request(h2_proxy_session *
apr_table_setn(r->notes, "proxy-source-port", apr_psprintf(r->pool, "%hu",
ctx->p_conn->connection->local_addr->port));
status = h2_proxy_session_submit(session, url, r);
- if (status != OK) {
+ if (status != APR_SUCCESS) {
ap_log_cerror(APLOG_MARK, APLOG_ERR, status, r->connection,
APLOGNO(03351)
"pass request body failed to %pI (%s) from %s (%s)",
ctx->p_conn->addr, ctx->p_conn->hostname ?
