WebDAV COPY/MOVE path containment and resource overwrite risk (improper path validation)

PoC (actionable steps): Assumptions: A DAV-enabled nginx instance at example.org has WebDAV enabled and contains a file at /dav/secret.txt. The Destination header is evaluated by the server during COPY/MOVE operations. 1) Copy to the same location (expected to be caught by the fix): Request: COPY /dav/secret.txt HTTP/1.1 Host: example.org Destination: http://example.org/dav/secret.txt Expected on vulnerable versions: The operation may proceed and could overwrite or otherwise mishandle the resource. After the fix, the server should respond with 403 Forbidden. 2) Copy into a parent/ancestor path (example of a prefix relationship): Request: COPY /dav/subdir/secret.txt HTTP/1.1 Host: example.org Destination: http://example.org/dav/subdir/ Expected on vulnerable versions: Depending on implementation details, this could lead to unintended containment/overwrite behavior. After the fix, the server should respond with 403 Forbidden. Notes: - Use actual WebDAV clients or HTTP tools that support the COPY method (e.g., curl -X COPY). - Ensure the test environment has appropriate backups and is isolated, as COPY/MOVE can overwrite or restructure resources.

diff --git a/src/http/modules/ngx_http_dav_module.c b/src/http/modules/ngx_http_dav_module.c index 4619b139a2..dd960ca271 100644 --- a/src/http/modules/ngx_http_dav_module.c +++ b/src/http/modules/ngx_http_dav_module.c @@ -47,6 +47,9 @@ static ngx_int_t ngx_http_dav_mkcol_handler(ngx_http_request_t *r, ngx_http_dav_loc_conf_t *dlcf); static ngx_int_t ngx_http_dav_copy_move_handler(ngx_http_request_t *r); +static void ngx_http_dav_merge_slashes(ngx_str_t *path); +static ngx_int_t ngx_http_dav_validate_paths(ngx_http_request_t *r, + ngx_str_t *src, ngx_str_t *dst, ngx_uint_t slash, ngx_table_elt_t *dest); static ngx_int_t ngx_http_dav_copy_dir(ngx_tree_ctx_t *ctx, ngx_str_t *path); static ngx_int_t ngx_http_dav_copy_dir_time(ngx_tree_ctx_t *ctx, ngx_str_t *path); @@ -719,6 +722,9 @@ ngx_http_dav_copy_move_handler(ngx_http_request_t *r) r->uri = uri; + ngx_http_dav_merge_slashes(&path); + ngx_http_dav_merge_slashes(&copy.path); + copy.path.len--; /* omit "\0" */ if (copy.path.data[copy.path.len - 1] == '/') { @@ -733,6 +739,12 @@ ngx_http_dav_copy_move_handler(ngx_http_request_t *r) ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "http copy to: \"%s\"", copy.path.data); + if (ngx_http_dav_validate_paths(r, &path, &copy.path, slash, dest) + != NGX_OK) + { + return NGX_HTTP_FORBIDDEN; + } + if (ngx_link_info(copy.path.data, &fi) == NGX_FILE_ERROR) { err = ngx_errno; @@ -870,6 +882,65 @@ ngx_http_dav_copy_move_handler(ngx_http_request_t *r) } +static void +ngx_http_dav_merge_slashes(ngx_str_t *path) +{ + u_char *p, *q; + + p = path->data; + q = path->data; + + while (*p) { + *q++ = *p; + + if (*p++ == '/') { + while (*p == '/') { + p++; + } + } + } + + *q++ = '\0'; + path->len = q - path->data; +} + + +static ngx_int_t +ngx_http_dav_validate_paths(ngx_http_request_t *r, ngx_str_t *src, + ngx_str_t *dst, ngx_uint_t slash, ngx_table_elt_t *dest) +{ + size_t len; + + len = src->len - 1; + + if (len > 0 && src->data[len - 1] == '/') { + len--; + } + + if (len == dst->len && ngx_strncmp(src->data, dst->data, len) == 0) { + ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, + "both URI \"%V\" and \"Destination\" URI \"%V\" " + "point to the same location", + &r->uri, &dest->value); + return NGX_HTTP_FORBIDDEN; + } + + if (slash + && ngx_strncmp(src->data, dst->data, ngx_min(len, dst->len)) == 0 + && (len < dst->len + ? dst->data[len] == '/' + : src->data[dst->len] == '/')) + { + ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, + "\"%V\" could not be %Ved to collection \"%V\"", + &r->uri, &r->method_name, &dest->value); + return NGX_HTTP_FORBIDDEN; + } + + return NGX_OK; +} + + static ngx_int_t ngx_http_dav_copy_dir(ngx_tree_ctx_t *ctx, ngx_str_t *path) {