|Summary:||[PATCH] Some PROPPATCH causing segfault in mod_dav_fs / mod_dav|
|Product:||Apache httpd-2||Reporter:||Diego Santa Cruz <Diego.SantaCruz>|
|Component:||mod_dav_fs||Assignee:||Apache HTTPD Bugs Mailing List <bugs>|
First patch to fix PROPPATCH segfault
Do not fail PROPPATCH when prop namespace not known
Do not segfault on PROPFIND with a zero length DBM
Description Diego Santa Cruz 2012-01-30 11:24:35 UTC
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.
Comment 1 Diego Santa Cruz 2012-01-30 11:31:18 UTC
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.
Comment 2 Diego Santa Cruz 2012-01-30 12:31:23 UTC
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).
Comment 3 Diego Santa Cruz 2012-01-30 12:43:07 UTC
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.
Comment 4 Diego Santa Cruz 2012-01-30 12:44:37 UTC
Forgot to mention, but the tests have been done against httpd 2.2.21. The attached patches are against this version too.
Comment 5 Wim Lewis 2013-04-17 18:57:10 UTC
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 ()
Comment 6 Wim Lewis 2013-04-22 22:22:43 UTC
The crash also still occurs in trunk/2.5 r1470659, no apparent change from 2.4.4
Comment 10 Graham Leggett 2013-04-27 19:07:54 UTC
Can you test and verify this works for you? Backport proposed for v2.4.
Comment 11 Graham Leggett 2013-05-12 10:30:31 UTC
Backported to v2.4.5.
Comment 12 Graham Leggett 2013-05-23 13:20:19 UTC
Proposed for backport to v2.2.
Comment 13 Wim Lewis 2015-03-11 23:52:16 UTC
Confirming I can no longer reproduce this in the 2.4.x series.