Skip to content

security: validate staged import file identifiers#808

Open
RussH wants to merge 3 commits into
masterfrom
security/import-temp-file-rce
Open

security: validate staged import file identifiers#808
RussH wants to merge 3 commits into
masterfrom
security/import-temp-file-rce

Conversation

@RussH

@RussH RussH commented Jun 11, 2026

Copy link
Copy Markdown
Member

Security: hardens import temporary file handling by validating staged import file identifiers before they are used to build paths under CATS_TEMP_DIR.

The import flow accepted a posted fileName value during staged imports. This change trims and validates the posted identifier before use, requiring it to:

  • be non-empty
  • match basename(fileID), preventing path traversal
  • exist in the server-side session whitelist of valid staged import file IDs
  • match using strict in_array() comparison

Invalid staged import file identifiers now return to the import screen before any file path is constructed or processed.

This was supplied as a git bundle for review rather than as a GitHub PR, so it was imported into a separate branch and reviewed before opening this PR.

Local checks performed:

  • git bundle verify passed
  • git fsck --full completed without corruption errors
  • branch contains two commits:
    • a9f4342 Harden import temporary file handling
    • ba0acbd Validate staged import file identifiers
  • changed file: modules/import/ImportUI.php
  • php -l modules/import/ImportUI.php passed
  • invalid fileName paths return before import processing continues

@RussH RussH requested a review from anonymoususer72041 June 11, 2026 15:40
@RussH RussH changed the title Security: validate staged import file identifiers security: validate staged import file identifiers Jun 11, 2026
@anonymoususer72041

anonymoususer72041 commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

@RussH please merge (#806 and) #807 first

@anonymoususer72041 anonymoususer72041 added the security Pull requests that address a security vulnerability label Jun 14, 2026

@anonymoususer72041 anonymoususer72041 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to drop the merge commit by rebasing instead of merging master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security Pull requests that address a security vulnerability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants