Skip to content
Open
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions ngx_http_auth_digest_module.c
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,32 @@ ngx_http_auth_digest_verify_hash(ngx_http_request_t *r,
ngx_md5_t md5;
u_char hash[16];

#ifdef NGX_HTTP_PROXY_CONNECT
if (r->method_name.len == 7 && ngx_strncmp(r->method_name.data, "CONNECT", 7) == 0) {
Copy link

Copilot AI Jun 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of comparing r->method_name bytes, consider using the NGINX request method enum (r->method == NGX_HTTP_CONNECT) for clarity and to avoid string comparisons.

Suggested change
if (r->method_name.len == 7 && ngx_strncmp(r->method_name.data, "CONNECT", 7) == 0) {
if (r->method == NGX_HTTP_CONNECT) {

Copilot uses AI. Check for mistakes.
// CONNECT requests don't have `r->unparsed_uri` set, so the URI must be validated
// against server address (& optionally port)
size_t uri_len = 0;
while (uri_len < fields->uri.len && fields->uri.data[uri_len++] != ':');
if (uri_len < fields->uri.len && fields->uri.data[uri_len] == ':') {
uri_len--;
}
if (!((r->connect_host.len == (uri_len - 1)) &&
(ngx_strncmp(r->connect_host.data, fields->uri.data,
uri_len) == 0))) {
Copy link

Copilot AI Jun 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The host-length check subtracts 1 from uri_len, likely causing an off-by-one error. Calculate the host segment length explicitly (e.g., host_len = colon_pos - uri_start) and compare against r->connect_host.len.

Suggested change
size_t uri_len = 0;
while (uri_len < fields->uri.len && fields->uri.data[uri_len++] != ':');
if (uri_len < fields->uri.len && fields->uri.data[uri_len] == ':') {
uri_len--;
}
if (!((r->connect_host.len == (uri_len - 1)) &&
(ngx_strncmp(r->connect_host.data, fields->uri.data,
uri_len) == 0))) {
size_t colon_pos = 0;
while (colon_pos < fields->uri.len && fields->uri.data[colon_pos] != ':') {
colon_pos++;
}
size_t host_len = colon_pos; // Host segment length is up to the colon
if (!((r->connect_host.len == host_len) &&
(ngx_strncmp(r->connect_host.data, fields->uri.data,
host_len) == 0))) {

Copilot uses AI. Check for mistakes.
return NGX_DECLINED;
}
if (uri_len + 1 < fields->uri.len && fields->uri.data[uri_len + 1] == ':') {
// need to check port as well
uri_len += 2; // skip `:` and position pointer at port
u_char* uri_port = fields->uri.data + uri_len;
size_t uri_port_len = fields->uri.len - uri_len;
if (!((uri_port_len != r->connect_port.len) &&
(ngx_strncmp(uri_port, r->connect_port.data, ngx_min(uri_port_len, r->connect_port.len)) == 0))) {
Copy link

Copilot AI Jun 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition uses uri_port_len != r->connect_port.len combined with && and negated, but it should reject if lengths differ OR contents differ. Replace with:

if (uri_port_len != r->connect_port.len
    || ngx_strncmp(uri_port, r->connect_port.data, uri_port_len) != 0) {
    return NGX_DECLINED;
}
Suggested change
if (!((uri_port_len != r->connect_port.len) &&
(ngx_strncmp(uri_port, r->connect_port.data, ngx_min(uri_port_len, r->connect_port.len)) == 0))) {
if (uri_port_len != r->connect_port.len ||
ngx_strncmp(uri_port, r->connect_port.data, uri_port_len) != 0) {

Copilot uses AI. Check for mistakes.
return NGX_DECLINED;
}
}
} else {
#endif
// The .net Http library sends the incorrect URI as part of the Authorization
// response. Instead of the complete URI including the query parameters it
// sends only the basic URI without the query parameters. It also uses this
Expand All @@ -660,6 +686,9 @@ ngx_http_auth_digest_verify_hash(ngx_http_request_t *r,
return NGX_DECLINED;
}
}
#ifdef NGX_HTTP_PROXY_CONNECT
}
#endif

// the hashing scheme:
// digest:
Expand Down