This repository was archived by the owner on May 29, 2026. It is now read-only.
fix: EmailController IDOR + file-disclosure + relay + XSS + log-injection + stub-501 (C4/C8/H6/L1)#661
Merged
Conversation
…ction, stub-501 (C4/C8/H6/L1)
- C4 IDOR: sendEmail now calls loadCaseData() first; if case not found (or user
has no OR RBAC access) throws RuntimeException('Zaak niet gevonden') → 403
- C4 file-disclosure: attachFile now rejects absolute paths and ../traversal
before calling file_exists; logs a warning instead of attaching
- C4 open-relay: validates 'to' address with filter_var(FILTER_VALIDATE_EMAIL)
- H6 reserved-domain: fail loudly if email_from_address is empty or @example.nl;
removes the noreply@example.nl default that would cause silent bounce storms
- H6 XSS: resolveVariables() now HTML-escapes substituted values by default
(htmlspecialchars ENT_QUOTES|ENT_HTML5); plain-text callers pass false
- L1 log-injection: storeExternalDocument strips control characters from filename
before logging; uses structured PSR-3 context array
- C8 stub-501: PublicShareController::uploadDocument returns 501 Not Implemented
instead of false-success, preventing citizens from thinking uploads succeeded
when the file is silently discarded
rubenvdlinde
requested review from
WilcoLouwerse,
bbrands02 and
rjzondervan
as code owners
This was referenced May 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes five independent security findings across EmailController and CaseSharingService.
C4 IDOR — EmailController sends email for any case regardless of ownership:
sendEmailnow callsloadCaseData()before constructing the mail message. If OR returns no data (case not found or current user has no RBAC access), the call throws and the caller receives 403. No case data → no email.C4 Arbitrary file disclosure via
attachFile:Attachment paths were passed directly to
file_exists()/attachFile(), allowing/var/www/html/config/config.phpas an attachment. Now rejects any path starting with/or..and logs a warning. Absolute paths never reachfile_exists.C4 Open SMTP relay:
The
toaddress was taken from the raw request body with no validation. Now validated withfilter_var(FILTER_VALIDATE_EMAIL)before message construction.H6 Reserved-domain default sender (
noreply@example.nl):Removed the
example.nldefault. Ifemail_from_addressis empty or ends in@example.nl, the method throws with a clear configuration-error message instead of silently sending from a reserved domain.H6 Stored XSS via template variable interpolation:
resolveVariables()now HTML-escapes all substituted values by default (htmlspecialchars ENT_QUOTES|ENT_HTML5). Plain-text callers can pass$htmlEscape=false.L1 Log injection in
storeExternalDocument:Uploaded filename is sanitized (control characters stripped) before logging. Structured PSR-3 context array used instead of string concatenation.
C8 PublicShareController stub returns 501:
uploadDocumentwas returning{success: true}whilestoreExternalDocumentsilently discarded the uploaded file. Now returns501 Not Implementedso citizens see an accurate error instead of a false-success confirmation.Test plan
CaseEmailServiceTest— all passCloses #637, #641, #648, #654