Skip to content

Harden cds get symlink protections: parent-directory symlinks, TOCTOU window, dangling-symlink test coverage #514

Description

@RonaldHensbergen

Context

Follow-up from non-blocking review comments on #509 (fix(cli): reject and never write through symlinked cds get destinations), which hardened cli/getter.py against a symlink planted at the leaf destination path.

Remaining gaps

  1. Symlinked parent directories are not guarded. _write_actions/mkdir(parents=True, exist_ok=True) only checks the destination leaf path for a symlink. If an ancestor directory (e.g. profiles/ itself) is a symlink, mkdir/shutil.copy2 will still write through it. (cli/getter.py:643, review comment fix(cli): reject and never write through symlinked cds get destinations #509 (comment))
  2. TOCTOU window between unlink() and copy2(). An attacker who replants a symlink in that narrow gap could still redirect the write. Full closure needs os.open(dest, O_WRONLY|O_CREAT|O_EXCL|O_NOFOLLOW) plus a manual byte copy instead of shutil.copy2.
  3. Test gap: existing symlink-guard tests in tests/test_getter.py only plant non-dangling symlinks (attack_target exists, then symlink_to). No test exercises a dangling symlink (target absent) even though the code comments claim "dangling or not" is handled. (review comment fix(cli): reject and never write through symlinked cds get destinations #509 (comment))

Suggested follow-up work

  • Guard ancestor directories of each destination for symlinks before mkdir(parents=True, ...).
  • Replace the unlink() + shutil.copy2() sequence with an atomic O_EXCL|O_NOFOLLOW open + manual byte copy to close the TOCTOU window.
  • Add a regression test that plants a dangling symlink at a destination path and asserts cds get rejects it the same way as a non-dangling one.

These were explicitly called out as non-blocking/out-of-scope for #509 (leaf-specific fix) and deferred to a follow-up.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:securitySecurity policies and checkspriority:lowNice to have, low urgency

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions