https://bz.apache.org/bugzilla/show_bug.cgi?id=69795
Bug ID: 69795
Summary: WebDAV quota bypass in mod_dav_fs COPY/MOVE due to
operator-precedence bug (quota.c)
Product: Apache httpd-2
Version: 2.5-HEAD
Hardware: PC
OS: Mac OS X 10.1
Status: NEW
Severity: normal
Priority: P2
Component: mod_dav_fs
Assignee: [email protected]
Reporter: [email protected]
Target Milestone: ---
I’m reporting what I believe to be a logic flaw in the WebDAV filesystem
repository provider (`mod_dav_fs`) that effectively disables quota enforcement
for COPY/MOVE operations. The issue is an operator-precedence bug that assigns
a boolean to `size` instead of the actual file size, so the over-quota
precondition almost never triggers.
### Affected component
- **Module**: `mod_dav_fs`
- **Function**: `dav_fs_quota_precondition`
- **File**: `modules/dav/fs/quota.c`
- **Introduced by**: commit `b75ba394e50c6dc2f56ff6912ce0088f77a03775` (Thu
Mar 2 15:46:12 2023 +0000), "Add RFC4331 quotas for mod_dav_fs" (file added)
- **Also touched**: commit `dffd4ae216042a6b2de1f8325c6af8664d89aa22` (Thu Mar
2 16:09:50 2023 +0000), "Add RFC4331 quotas for mod_dav_fs – Fix warnings" (did
not change the faulty expression)
- **Branches/tags containing introducing commit**: branch `trunk`; tag
`2.5.0-alpha2-ci-test-only` (per local repo metadata)
### Impact
- **What happens**: For WebDAV COPY/MOVE, the precondition intended to reject
over-quota operations assigns 0/1 to `size` instead of the number of bytes, so
the comparison against `available_bytes` is ineffective. Large copies/moves
proceed despite insufficient quota/space.
- **Primary risk**: Violation of configured quotas and potential disk
exhaustion (availability impact).
- **Scope**: Deployments with WebDAV enabled and writable locations where quota
preconditions are in effect (directory quota or FS available space mode).
### Root cause
Current code (note the precedence and resulting boolean assignment):
```c
/* modules/dav/fs/quota.c */
/* If source size is known, we can forecast ovequota */
if ((size = dav_fs_size(src) != DAV_FS_BYTES_ERROR) &&
(size > available_bytes)) {
status = HTTP_INSUFFICIENT_STORAGE;
*err = dav_new_error_tag(r->pool, status, 0, 0, msg, "DAV:", tag);
goto out;
}
```
`!=` binds tighter than `=`, so `dav_fs_size(src) != DAV_FS_BYTES_ERROR` is
evaluated first (yielding 0 or 1) and that boolean is assigned to `size`. With
positive `available_bytes`, `1 > available_bytes` is false, so the check never
fires.
Reference for return semantics used here:
```c
/* modules/dav/fs/repos.c */
apr_off_t dav_fs_size(const dav_resource *resource)
{
if ((resource->info->finfo.valid & APR_FINFO_SIZE))
return resource->info->finfo.size;
return DAV_FS_BYTES_ERROR; /* -1 */
}
```
### Suggested fix
Assign the actual size to `size` and compare against the sentinel and
`available_bytes`:
```diff
--- a/modules/dav/fs/quota.c
+++ b/modules/dav/fs/quota.c
@@ -339,12 +339,13 @@
/* If source size is known, we can forecast ovequota */
- if ((size = dav_fs_size(src) != DAV_FS_BYTES_ERROR) &&
- (size > available_bytes)) {
+ size = dav_fs_size(src);
+ if (size != DAV_FS_BYTES_ERROR &&
+ size > available_bytes) {
status = HTTP_INSUFFICIENT_STORAGE;
*err = dav_new_error_tag(r->pool, status, 0, 0,
msg, "DAV:", tag);
goto out;
}
```
An equivalent single-condition variant also works:
```c
if (((size = dav_fs_size(src)) != DAV_FS_BYTES_ERROR) && size >
available_bytes) { ... }
```
### Additional notes
- PUT path uses `Content-Length` and already compares `size > available_bytes`
correctly; the bug is specific to the COPY/MOVE branch.
- A quick tree-wide audit did not find other instances of the `(size = foo !=
ERR)` pattern.
- `git blame` for the affected lines (339–351 at time of writing) attributes
them to commit `b75ba394e50` by `manu` on 2023-03-02. The subsequent cleanup
commit `dffd4ae216` adjusted warnings and parentheses elsewhere but left this
expression unchanged.
### Suggested tests (high level)
- Configure a small quota (or rely on FS available space mode) on a writable
WebDAV collection.
- Create a source resource with size `S` where `S > available_bytes`.
- Attempt `COPY` or `MOVE` into the quota-limited location.
- Current behavior: operation is allowed.
- Expected with fix: 507 (Insufficient Storage) via
`dav_fs_quota_precondition`.
Thanks!
- Stanislav Fort (Aisle Research)
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
