Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix email header manipulation issues #17262

Merged
merged 1 commit into from
Sep 15, 2023
Merged

Fix email header manipulation issues #17262

merged 1 commit into from
Sep 15, 2023

Conversation

markstory
Copy link
Member

@markstory markstory commented Sep 8, 2023

If Message addresses are fed unvalidated user data, email recipients can be manipulated and other headers may be injected.

This issue and patch were authored by Waldemar Bartikowski who reported the issue via our security mailing list.

@markstory markstory added this to the 4.4.18 milestone Sep 8, 2023
@@ -999,7 +999,7 @@ protected function formatAddress(array $address): array
$return[] = $email;
} else {
$encoded = $this->encodeForHeader($alias);
if (preg_match('/[^a-z0-9+\-\\=? ]/i', $encoded)) {
if ($encoded === $alias && preg_match('/[^a-z0-9+\-\\=? ]/i', $encoded)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this reintroduces the vulnerability. mb_encode_mimeheader sometimes splits the encoded string into encoded and not encoded pieces: "test ?test" results in "test =?UTF-8?B?P3Rlc3Q=?=". I don't know how the resulting string would be handled correctly according to the RFCs.

Looking at the PHPMailer project it seems they have their own encoding method that always encodes the whole string, see https://github.com/PHPMailer/PHPMailer/blob/e88da8d679acc3824ff231fdc553565b802ac016/src/PHPMailer.php#L3591C21-L3591C39.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Encoding the entire address alias could be a solution for us as well then. I re-introduced this comparison because we would otherwise be putting encoded values in quoted strings which would then be treated as literal values and not encoded content.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a look at the PHPMailer implementation and it doesn't correctly handle a scenario that is important to the CakePHP community which is encoding email headers in ISO-2022-JP encoding. I also re-read the RFC 2047 and my initial interpretation was wrong. I've reverted my changes.

@markstory markstory force-pushed the email-header branch 2 times, most recently from 0afa021 to 4502ab2 Compare September 13, 2023 04:45
If `Message` addresses are fed unvalidated user data, email recipients
can be manipulated and other headers may be injected.

This issue and patch were authored by Waldemar Bartikowski who reported
the issue via our security mailing list.
@markstory markstory merged commit c92a3c0 into 4.x Sep 15, 2023
12 of 13 checks passed
@markstory markstory deleted the email-header branch September 15, 2023 02:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants