add length validation for Lock-Token to prevent underflow

modules/dav/main/mod_dav.c

@@ -3606,18 +3606,9 @@ static int dav_method_unlock(request_rec *r)
         return HTTP_BAD_REQUEST;
     }
 
-    locktoken_txt = apr_pstrdup(r->pool, const_locktoken_txt);
-    if (locktoken_txt[0] != '<') {
-        /* ### should provide more specifics... */
-        return HTTP_BAD_REQUEST;
-    }
-    locktoken_txt++;
-
-    if (locktoken_txt[strlen(locktoken_txt) - 1] != '>') {
-        /* ### should provide more specifics... */
-        return HTTP_BAD_REQUEST;
-    }
-    locktoken_txt[strlen(locktoken_txt) - 1] = '\0';
+    err = dav_parse_locktoken(r->pool, const_locktoken_txt, &locktoken_txt);
+    if (err != NULL)
+        return err->status;
 
     if ((err = (*locks_hooks->parse_locktoken)(r->pool, locktoken_txt,
                                                &locktoken)) != NULL) {
@@ -5522,4 +5513,4 @@ APR_IMPLEMENT_EXTERNAL_HOOK_RUN_FIRST(dav, DAV, int, method_precondition,
                                        dav_resource *src, const dav_resource *dest,
                                        const apr_xml_doc *doc,
                                        dav_error **err),
-                                       (r, src, dest, doc, err), DECLINED)
+                                       (r, src, dest, doc, err), DECLINED)

modules/dav/main/mod_dav.h

@@ -1331,6 +1331,9 @@ struct dav_hooks_propdb
 
 DAV_DECLARE(time_t) dav_get_timeout(request_rec *r);
 DAV_DECLARE(time_t) dav_get_timeout_string(request_rec *r, const char *s);
+DAV_DECLARE(dav_error *) dav_parse_locktoken(apr_pool_t *p,
+                                             const char *input,
+                                             char **output);
 
 /*
 ** Opaque, provider-specific information for a lock database.
@@ -2756,5 +2759,4 @@ DAV_DECLARE(const dav_resource_type_provider *) dav_get_resource_type_providers(
 #endif
 
 #endif /* _MOD_DAV_H_ */
-/** @} */
-
+/** @} */

modules/dav/main/ms_wdv.c

@@ -36,13 +36,21 @@ static void delete_if_fixup(request_rec *r)
     const char *if_hdr;
     const char *cp;
     apr_size_t len;
+    int has_open;
+    int has_close;
 
     if ((if_hdr =  apr_table_get(r->headers_in, "If")) == NULL)
         goto out;
 
     /* check for parenthesis enclosed value */
     len = strlen(if_hdr);
-    if (if_hdr[0] != '(' || if_hdr[len - 1]!= ')')
+    /*
+     * Guard len-based indexing to avoid reading if_hdr[len - 1]
+     * on empty input.
+     */
+    has_open = (len > 0 && if_hdr[0] == '(');
+    has_close = (len > 0 && if_hdr[len - 1] == ')');
+    if (!has_open || !has_close)
         goto out;
 
     for (cp = if_hdr; *cp; cp++) {
@@ -197,10 +205,13 @@ static dav_error *mswdv_combined_lock(request_rec *r)
      * section 4.5 suggests using Lock-Token without brakets.
      */
     if (lock_token_hdr) {
-        apr_size_t len = strlen(lock_token_hdr);
+        char *parsed_locktoken;
+
+        err = dav_parse_locktoken(r->pool, lock_token_hdr, &parsed_locktoken);
+        if (err != NULL)
+            goto out;
 
-        if (lock_token_hdr[0] == '<' || lock_token_hdr[len - 1] == '>')
-            lock_token_hdr = apr_pstrndup(r->pool, lock_token_hdr + 1, len - 2);
+        lock_token_hdr = parsed_locktoken;
     }
 
     if (lock_timeout_hdr) {
@@ -829,5 +840,4 @@ DAV_DECLARE(apr_status_t) dav_mswdv_input(ap_filter_t *f,
     APR_BRIGADE_CONCAT(bb, bbsub);
 
     return APR_SUCCESS;
-}
-
+}

modules/dav/main/util.c

@@ -590,6 +590,47 @@ DAV_DECLARE(time_t) dav_get_timeout_string(request_rec *r,
     return DAV_TIMEOUT_INFINITE;
 }
 
+DAV_DECLARE(dav_error *) dav_parse_locktoken(apr_pool_t *p,
+                                             const char *input,
+                                             char **output)
+{
+    char *token;
+    apr_size_t len;
+    int has_open;
+    int has_close;
+
+    if (output == NULL) {
+        return dav_new_error(p, HTTP_INTERNAL_SERVER_ERROR, 0, 0,
+                             "DAV lock token parser called with NULL output pointer.");
+    }
+    *output = NULL;
+
+    if (input == NULL) {
+        return dav_new_error(p, HTTP_BAD_REQUEST, 0, 0,
+                             "No Lock-Token specified in header.");
+    }
+
+    len = strlen(input);
+
+    has_open = (len > 0 && input[0] == '<');
+    has_close = (len > 0 && input[len - 1] == '>');
+
+    if (len < 2 || !has_open || !has_close) {
+        return dav_new_error(p, HTTP_BAD_REQUEST, 0, 0,
+                             "Malformed Lock-Token header.");
+    }
+
+    token = apr_pstrndup(p, input + 1, len - 2);
+
+    if (*token == '\0') {
+        return dav_new_error(p, HTTP_BAD_REQUEST, 0, 0,
+                             "Malformed Lock-Token header.");
+    }
+
+    *output = token;
+    return NULL;
+}
+
 /* ---------------------------------------------------------------
 **
 ** If Header processing
@@ -2228,4 +2269,4 @@ DAV_DECLARE(dav_error *) dav_auto_checkin(
     }
 
     return NULL;
-}
+}