[WIP] BB-3813 Remove Host-header-derived base tag from default email template#1
Draft
AlexArchiPro wants to merge 1 commit into
Draft
[WIP] BB-3813 Remove Host-header-derived base tag from default email template#1AlexArchiPro wants to merge 1 commit into
AlexArchiPro wants to merge 1 commit into
Conversation
The <% base_tag %> in Email.ss resolves via Director::absoluteBaseURL(), which is derived from the unvalidated Host header when no alternate base URL is configured. A poisoned Host header on a forgot-password request produced emails whose base tag pointed at the attacker's domain, allowing password reset link hijacking. Emails do not need a base tag: MailerSubscriber (registered by default in _config/mailer.yml) already rewrites all relative URLs in HTML and plain text bodies to absolute URLs via HTTP::absoluteURLs() before sending.
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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
The default email template
templates/SilverStripe/Control/Email/Email.ssemitted<% base_tag %>, which resolves throughDirector::absoluteBaseURL(). When noalternate_base_urlis configured, that value is derived from the unvalidatedHostrequest header. An attacker could POST a forgot-password request with a poisonedHostheader, producing an email whose<base>tag points at the attacker's domain so the relative password reset link resolves there — enabling reset link capture and account takeover.This removes the
<% base_tag %>from the default email template. Emails do not rely on it:MailerSubscriber(registered by default in_config/mailer.yml) rewrites all relative URLs in both HTML and plain-text email bodies to absolute URLs viaHTTP::absoluteURLs()before sending, so links such as the password reset link remain functional.<base>tags anyway, so the tag provided no reliable benefit while introducing the Host poisoning sink.<base>tag in rendered email output (the onlybase_tagreferences undertests/php/Control/Email/are test-local fixture templates simulating user templates, which are unaffected).Jira: https://archipro.atlassian.net/browse/BB-3813
Testing
composer installwith PHP 8.1, then ran the focused suite:vendor/bin/phpunit tests/php/Control/Email/EmailTest.php— 34 tests, 137 assertions, all test methods pass. There is onetearDownAfterClassfailure ("Access denied for user 'root'@'localhost'") caused by no MySQL server being available in this environment; verified it occurs identically on the unmodified branch-5 head, so it is pre-existing and unrelated to this change.