With our product we stumbled across a segfault in mod_dav_fs / mod_dav, as follows. When a PROPPATCH attempts to remove a non-existent dead property on a resource for which there is no dead property in the same namespace httpd segfaults (e.g., attempt to remove a property 'foo' in namespace 'http://example.com/bar' on a new resource which has no other dead properties). Furthermore, if the resource had no dead properties, after this segfault httpd leaves the dead property DBM in a state which causes an httpd segfault on every PROPFIND (the .pag and .dir files are both zero length). From our analysis the problem boils down to three issues. A patch for each follows.
Created attachment 28228 [details] First patch to fix PROPPATCH segfault This patch fixes the described segfault in the PROPPATCH. In the described case a rollback cannot be created when doing the dav_method_proppacth(), because dav_propdb_get_rollback() fails without returning a rollback object. Thus the call to dav_propdb_apply_rollback() segfaults because rollback is NULL. This patch just protects against that case considering it a success.
Created attachment 28229 [details] Do not fail PROPPATCH when prop namespace not known With the patch in attachment 28228 [details] the PROPPATCH described above no longer segfaults httpd, but fails with a 500 error. As stated in RFC 4918 (WebDAV), section 14.23, removing a non-existing property is not an error. The failure comes from dav_propdb_get_rollback(), which returns an error for the non-existent property. It turns out that if the property's namespace is not known (as can well be the case for a non-existent property) dav_build_key() returns a zeroed key, but doing a dav_dbm_fetch() on such a key returns an error. In fact, APR's apr_dbm_fetch() returns APR_EINVAL for such a key (at least the SDBM backend). The patch modifies dav_dbm_fetch() so that is catches this case and treats it as a "key not found", which is the actual error as the namespace index is built from the DBM. With this patch httpd succeds the test case described above (doing a PROPPATCH to remove a non-existing property on a resource which does not have any other property in the same namespace).
Created attachment 28230 [details] Do not segfault on PROPFIND with a zero length DBM As described above, when httpd segfaults during the PROPPATCH it leaves a zero length DBM if no other dead properties existed for the resource. Doing a PROPFIND on the resource segfaults httpd. The cause of the segfault is that dav_get_allprops() does not check the return value of the first_name() nor next_name() DB hooks for errors. When the DBM is of zero length (both the .dir and .pag files are zero length) first_name() returns an error and leaves its 'name' argument uninitialized. But then 'name.ns' is accessed just after the first_name() call, possibly causing a segfault or other errors as 'name' is stack allocated. The attached patch changes this so that the return value of first_name() and next_name() is checked and the while loop on the properties be stopped in case of error. As it seems that dav_get_allprops() cannot return an error I could not see another way to handle this situation and this is how errors on the output_value() hook call are treated within dav_get_allprops() anyhow.
Forgot to mention, but the tests have been done against httpd 2.2.21. The attached patches are against this version too.
I can confirm this crash still occurs in Apache 2.4.4: the PROPFIND causes a segfault and leaves the zero-length DAV prop db behind. (However, the zero-length prop db no longer causes later operations to crash the server, so that's progress I guess.) Program received signal EXC_BAD_ACCESS, Could not access memory. Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000010 0x0000000100602c53 in dav_propdb_apply_rollback () (gdb) bt #0 0x0000000100602c53 in dav_propdb_apply_rollback () #1 0x00000001000c8d2d in dav_prop_rollback () #2 0x00000001000c1b0e in dav_process_ctx_list () #3 0x00000001000c2074 in dav_method_proppatch () #4 0x00000001000c7063 in dav_handler () #5 0x00000001000015ef in ap_run_handler () #6 0x0000000100001eaf in ap_invoke_handler () #7 0x000000010005b7de in ap_process_async_request () #8 0x000000010005b8b0 in ap_process_request () #9 0x00000001000573fb in ap_process_http_sync_connection () #10 0x00000001000574f6 in ap_process_http_connection () #11 0x0000000100019906 in ap_run_process_connection () #12 0x0000000100019dd7 in ap_process_connection () #13 0x00000001000e24d8 in child_main () #14 0x00000001000e25e4 in make_child () #15 0x00000001000e2c5d in prefork_run () #16 0x000000010001c47d in ap_run_mpm () #17 0x000000010000d924 in main ()
The crash also still occurs in trunk/2.5 r1470659, no apparent change from 2.4.4
First patch applied to trunk in r1476642.
Second patch applied to trunk in r1476644.
Third patch applied to trunk in r1476645.
Can you test and verify this works for you? Backport proposed for v2.4.
Backported to v2.4.5.
Proposed for backport to v2.2.
Confirming I can no longer reproduce this in the 2.4.x series.