mod_dav: Fix NULL pointer dereference in ms_wdv.c check_locked_by_other()

modules/dav/main/ms_wdv.c

@@ -157,7 +157,8 @@ static dav_error *check_locked_by_other(request_rec *r)
     /* Let lock method fail the request */
 
 out:
-    (*lockdb->hooks->close_lockdb)(lockdb);
+    if (lockdb)
+        (*lockdb->hooks->close_lockdb)(lockdb);
 
     return err;
 }

test/pytest_suite/t/conf/extra.conf.in

@@ -656,6 +656,7 @@ Alias /manual @inherit_documentroot@/manual
 
     <Directory @SERVERROOT@/htdocs/modules/dav>
         DAV On
+        DAVMSext All
     </Directory>
 </IfModule>
 

test/pytest_suite/tests/t/modules/test_dav.py

@@ -207,3 +207,41 @@ def test_dav(http):
                 os.rmdir(p)
             except OSError:
                 pass
+
+
+@need_module("dav")
+def test_dav_mswdv_null_deref(http):
+    """BZ 70051: a PUT/DELETE/MOVE/LOCK whose dav_get_resource() fails (e.g. an
+    extraneous path component under an existing file) must not crash the
+    worker. Under DAVMSext, check_locked_by_other() in ms_wdv.c used to
+    dereference its lockdb unconditionally in the out: label, even on the
+    paths that goto out before open_lockdb() runs and leave it NULL.
+    """
+    htdocs = http.vars("documentroot")
+    os.makedirs(os.path.join(htdocs, DIR), exist_ok=True)
+    name = "mswdv-70051.html"
+    fpath = os.path.join(htdocs, DIR, name)
+    if os.path.exists(fpath):
+        os.unlink(fpath)
+
+    base = http.base_url
+    url = f"{base}/{DIR}/{name}"
+
+    with httpx.Client(timeout=30.0) as c:
+        resp = c.put(url, content=BODY)
+        assert resp.is_success, f"seed PUT failed (got {resp.status_code})"
+
+        # dav_fs_get_resource() rejects the extra path component below the
+        # plain file with an error -- dav_get_resource() fails before any
+        # lockdb is opened. Pre-fix, check_locked_by_other()'s "out:" label
+        # dereferenced that still-NULL lockdb and crashed the worker.
+        resp = c.put(f"{url}/extra", content="boom")
+        assert resp.status_code in (403, 404, 409), \
+            f"PUT past a file should be a clean DAV error, got {resp.status_code}"
+
+        # the worker must still be alive: a normal request must succeed.
+        resp = c.get(url)
+        assert resp.status_code == 200, \
+            f"server did not survive the request (got {resp.status_code})"
+
+    os.unlink(fpath)