Skip to content

security: Enforce candidate/attachment access checks and centralize authorization logic#828

Open
RussH wants to merge 1 commit into
masterfrom
codex/fix-authorization-check-for-hidden-candidates
Open

security: Enforce candidate/attachment access checks and centralize authorization logic#828
RussH wants to merge 1 commit into
masterfrom
codex/fix-authorization-check-for-hidden-candidates

Conversation

@RussH

@RussH RussH commented Jul 9, 2026

Copy link
Copy Markdown
Member

Motivation

  • Centralize and enforce object-level access checks for candidates and candidate attachments to prevent unauthorized access to hidden candidates and related attachments.
  • Reduce duplicated permission checks across candidate and attachment code paths and make access logic reusable.

Description

  • Add lib/CandidateAuthorization.php providing getCandidate, canAccessCandidate, getAttachment, and canAccessCandidateAttachment helpers for candidate and attachment access validation.
  • Update modules/attachments/AttachmentsUI.php to validate access to candidate attachments via CandidateAuthorization::canAccessCandidateAttachment before serving files.
  • Update modules/candidates/CandidatesUI.php to include CandidateAuthorization and add helper methods enforceCandidateHiddenAccess and enforceAttachmentCandidateHiddenAccess, and use them throughout (edit, delete, add/remove pipeline, activities, tags, attachment create/delete, resume view, questionnaires, image/attachment modals, duplicate handling, etc.) to validate access and fail with a permission error when needed.
  • Update modules/companies/CompaniesUI.php to add helpers getCompanyForAction and getCompanyAttachmentForAction to validate company and company-attachment inputs used by attachment endpoints.
  • Update modules/lists/ajax/addToLists.php to check candidate access for items being added to saved lists when the dataItemType is DATA_ITEM_CANDIDATE.

Testing

  • Performed PHP syntax checks (php -l) on the modified files to ensure no syntax errors were introduced; no syntax errors reported.
  • No new automated unit tests were added for this change, and existing automated test suite was exercised against the changes with no regressions observed in smoke testing.

Codex Task

@RussH RussH changed the title Enforce candidate/attachment access checks and centralize authorization logic security: Enforce candidate/attachment access checks and centralize authorization logic Jul 9, 2026
@RussH RussH requested a review from anonymoususer72041 July 9, 2026 16:44
@anonymoususer72041

Copy link
Copy Markdown
Contributor

@mattmumford-git could you take a look onto this? Will this solve your advisories?

I also suggest to create one big advisory instead of multiple smaller ones as they will like not get assigned multiple CVEs.

@mattmumford-git

Copy link
Copy Markdown

Thanks for taking a look at this.

At a high level, yes, centralising the candidate and attachment authorization logic sounds like the right approach and should address the reports I submitted, provided the checks are enforced before each object read/write/delete action rather than only in the UI flow.

The key things that would need validating are:

  • Non-Site-Administrator users remain blocked from viewing hidden candidates.
  • Direct requests to hidden candidate actions now fail with a permission error and do not modify the database.
  • Candidate attachment access is checked by resolving the owning candidate from the attachment record, not by trusting a submitted candidateID.
  • Site boundaries are still enforced.
  • The saved-list AJAX flow rejects hidden candidate IDs when dataItemType is DATA_ITEM_CANDIDATE.
  • The affected endpoints I reported all return permission errors when replaying the PoCs: resume/attachment access, attachment delete, candidate edit/delete, pipeline actions, activity/tag actions, attachment/image creation, and saved-list add.

On the advisory structure, that makes sense. I reported them separately to keep the individual impact and proof of concept for each endpoint clear, but I’m happy for the project to consolidate them into one broader advisory for the hidden-candidate object authorization class if that fits the project and CVE process better.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants