From 70f6220c50d39a230dfdb67166ac5da4c159a850 Mon Sep 17 00:00:00 2001 From: Sagar Chanchal Date: Thu, 27 Aug 2026 18:41:26 +0530 Subject: [PATCH] Strip CR/LF from SMTP subjects derived from event data The SmtpAppender subject is produced by a PatternLayout serializer and commonly embeds event data (message, MDC, throwable). CR/LF sequences in that data flow into MimeMessage.setSubject unsanitized, allowing mail header injection when an attacker can influence logged content (for example a 'Bcc' header relayed through the application's SMTP credentials). Strip CR and LF from subjects in MimeMessageBuilder.setSubject and in the SmtpManager multipart send paths of both the javax (log4j-core) and jakarta (log4j-jakarta-smtp) modules. Signed-off-by: Sagar Chanchal --- .../core/net/MimeMessageBuilderTest.java | 50 +++++++++++++++++++ .../log4j/core/net/MimeMessageBuilder.java | 10 +++- .../logging/log4j/core/net/SmtpManager.java | 10 +++- .../log4j/smtp/MimeMessageBuilder.java | 10 +++- .../logging/log4j/smtp/SmtpManager.java | 10 +++- 5 files changed, 86 insertions(+), 4 deletions(-) create mode 100644 log4j-core-test/src/test/java/org/apache/logging/log4j/core/net/MimeMessageBuilderTest.java diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/net/MimeMessageBuilderTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/net/MimeMessageBuilderTest.java new file mode 100644 index 00000000000..4388d4f4e08 --- /dev/null +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/net/MimeMessageBuilderTest.java @@ -0,0 +1,50 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache license, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the license for the specific language governing permissions and + * limitations under the license. + */ +package org.apache.logging.log4j.core.net; + +import java.util.Properties; + +import javax.mail.Session; +import javax.mail.internet.MimeMessage; + +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests {@link MimeMessageBuilder} header sanitization. + */ +class MimeMessageBuilderTest { + + @Test + void testSubjectWithoutLineBreaksIsUnchanged() throws Exception { + MimeMessageBuilder builder = new MimeMessageBuilder(Session.getInstance(new Properties())); + MimeMessage message = builder.setSubject("Error in app").build(); + assertThat(message.getSubject()).isEqualTo("Error in app"); + } + + @Test + void testSubjectLineBreaksAreStripped() throws Exception { + MimeMessageBuilder builder = new MimeMessageBuilder(Session.getInstance(new Properties())); + MimeMessage message = builder.setSubject("Error\r\nBcc: victim@example.com").build(); + // CR and LF are stripped, so the remainder is inert text within the single + // subject value and cannot start a new header. + assertThat(message.getSubject()) + .doesNotContain("\r") + .doesNotContain("\n"); + } +} diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/MimeMessageBuilder.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/MimeMessageBuilder.java index 7e37afcce39..7249e6b027f 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/MimeMessageBuilder.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/MimeMessageBuilder.java @@ -71,11 +71,19 @@ public MimeMessageBuilder setRecipients(final Message.RecipientType recipientTyp public MimeMessageBuilder setSubject(final String subject) throws MessagingException { if (subject != null) { - message.setSubject(subject, StandardCharsets.UTF_8.name()); + message.setSubject(sanitizeHeader(subject), StandardCharsets.UTF_8.name()); } return this; } + /** + * Removes CR and LF characters from the value of a header set from (possibly + * attacker-controlled) event data, to prevent header injection. + */ + private static String sanitizeHeader(final String value) { + return value.indexOf('\r') >= 0 || value.indexOf('\n') >= 0 ? value.replaceAll("[\\r\\n]", " ") : value; + } + /** * @deprecated Use {@link #build()}. */ diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/SmtpManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/SmtpManager.java index 72376d39650..8701124ac6a 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/SmtpManager.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/SmtpManager.java @@ -259,11 +259,19 @@ protected void sendMultipartMessage(final MimeMessage msg, final MimeMultipart m synchronized (msg) { msg.setContent(mp); msg.setSentDate(new Date()); - msg.setSubject(subject); + msg.setSubject(sanitizeHeader(subject)); Transport.send(msg); } } + /** + * Removes CR and LF characters from the value of a header set from (possibly + * attacker-controlled) event data, to prevent header injection. + */ + private static String sanitizeHeader(final String value) { + return value.indexOf('\r') >= 0 || value.indexOf('\n') >= 0 ? value.replaceAll("[\\r\\n]", " ") : value; + } + private synchronized void connect(final LogEvent appendEvent) { if (message != null) { return; diff --git a/log4j-jakarta-smtp/src/main/java/org/apache/logging/log4j/smtp/MimeMessageBuilder.java b/log4j-jakarta-smtp/src/main/java/org/apache/logging/log4j/smtp/MimeMessageBuilder.java index d21fede2025..0a6e3c0a5fd 100644 --- a/log4j-jakarta-smtp/src/main/java/org/apache/logging/log4j/smtp/MimeMessageBuilder.java +++ b/log4j-jakarta-smtp/src/main/java/org/apache/logging/log4j/smtp/MimeMessageBuilder.java @@ -71,11 +71,19 @@ public MimeMessageBuilder setRecipients(final Message.RecipientType recipientTyp public MimeMessageBuilder setSubject(final String subject) throws MessagingException { if (subject != null) { - message.setSubject(subject, StandardCharsets.UTF_8.name()); + message.setSubject(sanitizeHeader(subject), StandardCharsets.UTF_8.name()); } return this; } + /** + * Removes CR and LF characters from the value of a header set from (possibly + * attacker-controlled) event data, to prevent header injection. + */ + private static String sanitizeHeader(final String value) { + return value.indexOf('\r') >= 0 || value.indexOf('\n') >= 0 ? value.replaceAll("[\\r\\n]", " ") : value; + } + @Override public MimeMessage build() { return message; diff --git a/log4j-jakarta-smtp/src/main/java/org/apache/logging/log4j/smtp/SmtpManager.java b/log4j-jakarta-smtp/src/main/java/org/apache/logging/log4j/smtp/SmtpManager.java index f98457a4102..23565eb2acb 100644 --- a/log4j-jakarta-smtp/src/main/java/org/apache/logging/log4j/smtp/SmtpManager.java +++ b/log4j-jakarta-smtp/src/main/java/org/apache/logging/log4j/smtp/SmtpManager.java @@ -212,11 +212,19 @@ protected void sendMultipartMessage(final MimeMessage msg, final MimeMultipart m synchronized (msg) { msg.setContent(mp); msg.setSentDate(new Date()); - msg.setSubject(subject); + msg.setSubject(sanitizeHeader(subject)); Transport.send(msg); } } + /** + * Removes CR and LF characters from the value of a header set from (possibly + * attacker-controlled) event data, to prevent header injection. + */ + private static String sanitizeHeader(final String value) { + return value.indexOf('\r') >= 0 || value.indexOf('\n') >= 0 ? value.replaceAll("[\\r\\n]", " ") : value; + } + private synchronized void connect(final LogEvent appendEvent) { if (message != null) { return;