Skip to content

Dav: improved path validation for COPY and MOVE operations#1307

Merged
saikrishnakumarreddy merged 1 commit into
nginx:masterfrom
saikrishnakumarreddy:fix/dav-self-copy
May 6, 2026
Merged

Dav: improved path validation for COPY and MOVE operations#1307
saikrishnakumarreddy merged 1 commit into
nginx:masterfrom
saikrishnakumarreddy:fix/dav-self-copy

Conversation

@saikrishnakumarreddy
Copy link
Copy Markdown
Contributor

@saikrishnakumarreddy saikrishnakumarreddy commented Apr 30, 2026

Improved path validation for COPY and MOVE operations

The COPY and MOVE handler did not validate whether source and
destination paths referred to the same resource or a parent-child
collection relationship, which could corrupt or destroy files.

Now 403 is returned if paths match or one is a prefix of the other.

Testing

Describe how you tested the change (manual testing, automated tests,
regression tests, etc.).

Checklist

Before submitting this PR, please confirm:

  • I have read the CONTRIBUTING guidelines
  • All existing tests pass
  • My branch is rebased on the latest master
  • This PR targets the master branch from my fork
  • My commit message follows NGINX standards (imperative mood, clear
    subject, references related issue if applicable, and contains only
    relevant changes)

@pluknet
Copy link
Copy Markdown
Contributor

pluknet commented Apr 30, 2026

The commit message could be less verbose for such a change, but it's good this way too.
Some nitpicking on it.

Dav: added path validation for COPY and MOVE operations

improved?

Because certain validation was already there.

Fix: after resolving ...

Such wording is not used, I suggest to change it to something like:

Now, after resolving ...

or

The fix is, after resolving ...

Otherwise, looks good.

@saikrishnakumarreddy saikrishnakumarreddy force-pushed the fix/dav-self-copy branch 2 times, most recently from 5c3916b to ac93c38 Compare May 4, 2026 07:03
@saikrishnakumarreddy saikrishnakumarreddy changed the title Dav: added path validation for COPY and MOVE operations Dav: improved path validation for COPY and MOVE operations May 4, 2026
@pluknet
Copy link
Copy Markdown
Contributor

pluknet commented May 4, 2026

The checks don't catch a path name with multiple consecutive slashes, which maps to the same file.
What we can do here is to merge slashes then compare, or write such a compare function that would take this into account.

@saikrishnakumarreddy saikrishnakumarreddy force-pushed the fix/dav-self-copy branch 4 times, most recently from 0f8d70a to a89a7f8 Compare May 5, 2026 12:06
Comment thread src/http/modules/ngx_http_dav_module.c Outdated
Comment thread src/http/modules/ngx_http_dav_module.c Outdated
Comment thread src/http/modules/ngx_http_dav_module.c Outdated
Comment thread src/http/modules/ngx_http_dav_module.c Outdated
Comment thread src/http/modules/ngx_http_dav_module.c Outdated
Comment thread src/http/modules/ngx_http_dav_module.c Outdated
Comment thread src/http/modules/ngx_http_dav_module.c Outdated
Comment thread src/http/modules/ngx_http_dav_module.c Outdated
Comment thread src/http/modules/ngx_http_dav_module.c Outdated
Comment thread src/http/modules/ngx_http_dav_module.c Outdated
The COPY and MOVE handler did not validate whether source and
destination paths referred to the same resource or a parent-child
collection relationship, which could corrupt or destroy files.

Now 403 is returned if paths match or one is a prefix of the other.

Reported by Mufeed VH of Winfunc Research.
@saikrishnakumarreddy saikrishnakumarreddy merged commit f0a0846 into nginx:master May 6, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants