fix: harden calendar public sendmail against phishing (OC10-123)

[DeepDiver1975]

Summary

Hardens the public calendar "send mail" endpoint (POST /v1/public/sendmail,
EmailController::sendEmailPublicLink) against being abused to send phishing
mail from the ownCloud server identity. Ref: OC10-123.

Two independent issues let a low-privileged authenticated user craft a
convincing phishing mail (attacker-controlled recipient, link target and
injected body content), sent from the instance's configured From address:

1. HTML injection in the mail body — the calendar name was concatenated
   raw into a print_unescaped() block in mail.publication.html.php, so
   markup supplied via the name parameter (e.g. an <a href> phishing link)
   was emitted unescaped. The bold formatting is now resolved on the
   translator-controlled template string first, and the user-supplied
   name/username are passed through \OCP\Util::sanitizeHTML() before
   interpolation. The intended bold styling is preserved.

2. Unvalidated link target — the url parameter was accepted verbatim, so
   the "Click here to access it" button could point anywhere. url is now
   required to be an absolute http(s) URL located below this instance's
   calendar app base and matching a public-sharing route (p/embed/public

   • token), mirroring appinfo/routes.php. Anything else is rejected with
     400 Bad Request and no mail is sent.

Tests

tests/unit/controller/emailcontrollerTest.php:

• valid same-origin public link is accepted and a mail is sent;
• injected markup in name is escaped in the rendered body;
• foreign host, missing scheme, javascript:, wrong app and non-public routes
  are all rejected (data provider).

Full calendar unit suite passes locally (63 tests).

Notes / follow-up

This closes the phishing/HTML-injection vector reported. A residual,
lower-severity behaviour remains: an authenticated user can still email a link
to a legitimately public calendar to an arbitrary recipient. Fully binding
the recipient/link to a calendar the caller actually published would require a
new public (OCP) API in core to resolve a public token to its owner (the
token→owner mapping lives in the dav app's private CalDavBackend), which is
out of scope for this app-level fix.

🤖 Generated with Claude Code

[jvillafanez]

Do we need to consider something like public/../token?

[DeepDiver1975]

Code review + fixes applied

Reviewed the phishing-hardening changes. The escaping fix and overall approach are correct; I verified OC_L10N_String::__toString returns the text verbatim when called with no parameters, so resolving the bold markers on the untranslated string and then vsprintf-ing sanitizeHTML()-escaped values is sound — no double substitution, and the href is emitted via p() so a validated URL can't break out of the attribute.

I found two issues and pushed a fix (e775f09):

🔴 URL validation rejected legitimate links under an active front controller

isValidPublicCalendarUrl() compared the client URL against linkToRouteAbsolute('calendar.view.index'). Core's Router only injects /index.php when the front controller is inactive (lib/private/Route/Router.php:75-76), whereas the JS client always builds the public link with /index.php/ via OC.linkTo/OC.filePath (core/js/js.js:187, which is not front-controller-aware). So on a pretty-URL instance (front_controller_active=true):

• client sends https://host/index.php/apps/calendar/p/<token>
• server base is https://host/apps/calendar/
• strpos() prefix check fails → every legitimate mail rejected with 400

The original unit test masked this because it stubbed linkToRouteAbsolute to a fixed /index.php/... base. Fix normalises the /index.php/ segment away on both sides before matching, and I added a regression test (testEmailPublicLinkAcceptsValidUrlWithFrontController) that stubs the base without index.php.

🟡 Path-traversal within the same host

The prefix + ~^(p|embed|public)/[^/?#]+~ check left the rest of the path unvalidated, so .../p/tok/../../../../evil passed and a browser would normalise it to a different location. Host is pinned by the prefix, so this is same-origin only (low severity), but it defeats the "must be a public calendar link" intent. Fix rejects any .. segment; added to the provideRejectedUrls data provider.

🟡 Null user

getUser()->getDisplayName() was dereferenced before any check (pre-existing). Added a null guard returning 401.

Verification

Full calendar unit suite green locally: 65 tests, 366 assertions (was 63; +2 new tests).

Nothing else blocking. 👍

[DeepDiver1975]

Follow-up: deeper security + test review (eeb29ea)

Ran a dedicated offensive-security pass and a test-coverage pass over the current state. Both confirmed the HTML escaping and host-pinning are robust (userinfo @evil.com, protocol-relative, lookalike-suffix hosts, javascript:, CRLF into recipient/subject, and the /index.php/ normalization were all verified secure). One real residual and several coverage gaps remained — fixed here.

🟠 [MEDIUM] Residual traversal bypass in isValidPublicCalendarUrl()

The .. guard only matched slash-delimited segments, so:

• backslash payloads slipped through — browsers normalise \→/ client-side before sending, so .../p/x\..\..\..\index.php\s\TOKEN needs no server cooperation; and
• percent-encoded %2e%2e slipped through; and
• the route check was a prefix match (~^(p|embed|public)/[^/?#]+~), leaving a trailing path / ? / # unvalidated.

Net: a "valid" link could resolve to an arbitrary path on the same host, defeating the "must be a public calendar link" intent. Host stays pinned (no external redirect), so same-origin only — MEDIUM.

Fix:

• reject any URL containing a backslash;
• rawurldecode() the path once before re-checking for ..;
• fully anchor the route grammar — ~^(p/[^/?#]+(/[^/?#]+)?|(embed|public)/[^/?#]+)$~ — so no trailing path/query/fragment is allowed.

🧪 Test coverage

• Hardened the escaping test with a positive-render assertion (body must contain the real public link) so it can't pass vacuously on an empty/failed render.
• Assert embed/ and public/ routes are accepted (previously only p/ was).
• Cover the null-user → 401 branch.
• Added display-name (username) injection escaping test.
• Extended the rejected-URL provider: host-confusion (lookalike suffix, prefix-without-separator, protocol-relative), empty token, backslash + encoded traversal, and query/fragment tails.

Not changed (verified)

• The test-analyzer suggested mocking OCP\Mail\IMessage — but there's no such interface in this core version (lib/public/Mail/ has only IMailer), so mocking the concrete \OC\Mail\Message is correct.
• Arbitrary-recipient-for-a-valid-link remains the documented out-of-scope residual.

Verification

Full calendar unit suite green: 80 tests, 426 assertions (was 65). Both changed files lint clean; commit signed + DCO.

[DeepDiver1975]

Superseded by #1363, which fixes the phishing link vector at the trust boundary rather than by validating an attacker-supplied URL.

Instead of accepting a verbatim url and inspecting it (the approach here, which needed three bypass patches and stayed fragile), the API contract changes to take the public sharing token and the backend generates the link via IURLGenerator::linkToRouteAbsolute() — same-origin and a real public route by construction. isValidPublicCalendarUrl() is deleted entirely. The HTML-escaping half of this PR is retained in #1363.

Suggest closing this in favour of #1363.