https://bz.apache.org/bugzilla/show_bug.cgi?id=70299
Bug ID: 70299
Summary: mod_dav_lock: dav_generic_load_lock_record() reads the
indirect-lock key length as a 4-byte int, but the save
path always writes it as a full apr_size_t
Product: Apache httpd-2
Version: 2.4-HEAD
Hardware: PC
OS: Linux
Status: NEW
Severity: normal
Priority: P2
Component: mod_dav_lock
Assignee: [email protected]
Reporter: [email protected]
Target Milestone: ---
Found while reviewing the fix for BZ 70298 (dav_size_indirect() under-reserving
space, PR at https://github.com/apache/httpd/pull/905). This is a separate bug
in the same read/write pair, not touched by that PR.
modules/dav/lock/locks.c, dav_generic_save_lock_record() (the write path),
always writes the indirect-lock key length as a full-width field:
memcpy(ptr, &ip->key.dsize, sizeof(ip->key.dsize));
ip->key.dsize is apr_size_t, so this writes sizeof(apr_size_t) bytes (8 on a
typical LP64 build).
dav_generic_load_lock_record() (the read path), locks.c around line 670, reads
that same field back with a 4-byte int cast instead of matching the write
width:
ip->key.dsize = *((int *) (val.dptr + offset));
offset += sizeof(ip->key.dsize);
The read takes only the first 4 bytes of the 8-byte field, but offset is still
advanced by sizeof(ip->key.dsize) (8), matching the real field width. On a
little-endian LP64 system this happens to read the correct value for any
realistic (small, positive) key length, because the low-order 4 bytes of an
8-byte little-endian integer are exactly those bytes. On a big-endian system
the same cast would read the high-order 4 bytes, which are zero for any such
value, so every indirect lock's key would be read back with a length of 0.
For comparison, the equivalent code in modules/dav/fs/lock.c (a different lock
provider) reads this field correctly with a full-width memcpy:
memcpy(&ip->key.dsize, val.dptr + offset, sizeof(ip->key.dsize));
I have not reproduced this on an actual big-endian system or build; this is a
static read of the code, same as the sizing bug in BZ 70298. The generic lock
provider in modules/dav/lock/ is not wired into any module shipped with httpd
(confirmed by grepping the tree for dav_hooks_locks_generic /
ap_lookup_provider(..., "dav-lock", ...): only modules/dav/lock/ itself
references it), so this is low practical severity, same caveat as BZ 70298.
Suggested fix: read the field with a width-matching memcpy, as
modules/dav/fs/lock.c already does, instead of the int cast.
--
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]
