get_propfind_properties mutates PROPFIND_PROPERTIES in place: property list grows 7 entries per call, producing multi-MB PROPFIND bodies

[ciberkids]

Summary

get_propfind_properties() mutates the module-level PROPFIND_PROPERTIES list in place, so the requested-property list grows by 7 entries on every call. In a long-running process this produces multi-megabyte PROPFIND bodies that can take down the Nextcloud server being talked to.

nc_py_api/files/_files.py:

def get_propfind_properties(capabilities: dict) -> list:
    r = PROPFIND_PROPERTIES                 # aliases the module-level list, no copy
    if not check_capabilities("files.locking", capabilities):
        r += PROPFIND_LOCKING_PROPERTIES    # list.__iadd__ -> in-place extend
    return r

r = PROPFIND_PROPERTIES binds the same object, and += on a list is __iadd__, which extends in place. So the seven nc:lock-* properties are appended to the shared constant permanently, once per call.

Still present on main as of 0.30.2.

Reproduction

from nc_py_api.files._files import PROPFIND_PROPERTIES, get_propfind_properties

caps = {"files": {"locking": "1.0"}}
print(len(PROPFIND_PROPERTIES))        # 17
for i in range(5):
    get_propfind_properties(caps)
    print(len(PROPFIND_PROPERTIES))    # 24, 31, 38, 45, 52

Only affects servers advertising files.locking; without it the branch is skipped and nothing grows.

Impact

Every FilesAPI.listdir() calls this, and find() / by_id() reach it via build_find_request(), so any client doing repeated directory operations accumulates. Observed in production against Nextcloud 34 (PHP-FPM pm.max_children=6): the lock-property block was repeated 138,575 times in a single PROPFIND, a ~19 MB request body.

Server cost is O(requested_properties × resources), so each such request pinned a PHP-FPM worker at 100% CPU. With all six workers pinned the Nextcloud web login was starved and returned 504. Later runs got 413 Request Entity Too Large and retries re-sent the same body. Effectively a single well-behaved client DoS'ing its own server.

Second instance of the same bug

FilesAPI.trashbin_list() mutates the same shared object:

properties = PROPFIND_PROPERTIES
properties += ["nc:trashbin-filename", "nc:trashbin-original-location", "nc:trashbin-deletion-time"]

AsyncFilesAPI.trashbin_list() has it too. These add 3 per call to the same list that get_propfind_properties is growing by 7.

Suggested fix

def get_propfind_properties(capabilities: dict) -> list:
    r = list(PROPFIND_PROPERTIES)
    if not check_capabilities("files.locking", capabilities):
        r += PROPFIND_LOCKING_PROPERTIES
    return r

and in both trashbin_list implementations:

properties = [*PROPFIND_PROPERTIES, "nc:trashbin-filename", "nc:trashbin-original-location", "nc:trashbin-deletion-time"]

A regression test asserting len(PROPFIND_PROPERTIES) is unchanged after N calls would pin it. Worth noting for anyone patching downstream: files.py and files_async.py both do from ._files import get_propfind_properties, so they hold independent name bindings — monkeypatching _files alone does not fix listdir().

Happy to open a PR if that is useful.

Environment

• nc-py-api 0.30.1 and 0.30.2, Python 3.12
• Nextcloud 34.0.1 (linuxserver image), files_lock enabled

Reported downstream at ciberkids/cloud-drive-sync#47, which has the full incident detail.