You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by sw...@apache.org on 2026/05/16 05:25:03 UTC
(logging-log4cxx) branch master updated: Sanitize CRLF characters in SMTPAppender header fields (#672)
This is an automated email from the ASF dual-hosted git repository.
swebb2066 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
The following commit(s) were added to refs/heads/master by this push:
new 22a5fc82 Sanitize CRLF characters in SMTPAppender header fields (#672)
22a5fc82 is described below
commit 22a5fc82c884b928b0b2c3841826c21cc1046505
Author: metsw24-max <me...@gmail.com>
AuthorDate: Sat May 16 10:54:59 2026 +0530
Sanitize CRLF characters in SMTPAppender header fields (#672)
---
src/main/cpp/smtpappender.cpp | 43 ++++++++++++++++---
src/test/cpp/net/smtpappendertestcase.cpp | 71 +++++++++++++++++++++++++++++++
2 files changed, 109 insertions(+), 5 deletions(-)
diff --git a/src/main/cpp/smtpappender.cpp b/src/main/cpp/smtpappender.cpp
index 535e3916..496c3d53 100644
--- a/src/main/cpp/smtpappender.cpp
+++ b/src/main/cpp/smtpappender.cpp
@@ -44,6 +44,39 @@ using namespace LOG4CXX_NS::spi;
#include <libesmtp.h>
#endif
+namespace
+{
+// RFC 5322 §2.1 defines header fields as CRLF-terminated lines, so an embedded
+// CR or LF in a configured Subject/From/To/Cc/Bcc value would split it across
+// header boundaries on the wire — a caller who controls a configured field
+// (e.g. through ${...} property substitution from an environment variable)
+// could inject arbitrary additional headers such as Bcc. The library already
+// owns SMTP wire-format sanitization (see SMTPSession::toAscii, which silently
+// rewrites non-ASCII to '?'); strip CR/LF in the public setters so the same
+// boundary is enforced regardless of how the value reaches the appender.
+LogString stripSmtpControl(const LogString& value, const logchar* field)
+{
+ if (value.find_first_of(LOG4CXX_STR("\r\n")) == LogString::npos)
+ {
+ return value;
+ }
+ LogString warning(LOG4CXX_STR("SMTPAppender "));
+ warning.append(field);
+ warning.append(LOG4CXX_STR(" contains CR or LF; stripping to prevent SMTP header injection."));
+ LogLog::warn(warning);
+ LogString out;
+ out.reserve(value.size());
+ for (auto ch : value)
+ {
+ if (ch != 0x0D && ch != 0x0A)
+ {
+ out.append(1, ch);
+ }
+ }
+ return out;
+}
+} // namespace
+
namespace LOG4CXX_NS
{
namespace net
@@ -451,7 +484,7 @@ LogString SMTPAppender::getFrom() const
void SMTPAppender::setFrom(const LogString& newVal)
{
- _priv->from = newVal;
+ _priv->from = stripSmtpControl(newVal, LOG4CXX_STR("From"));
}
@@ -462,7 +495,7 @@ LogString SMTPAppender::getSubject() const
void SMTPAppender::setSubject(const LogString& newVal)
{
- _priv->subject = newVal;
+ _priv->subject = stripSmtpControl(newVal, LOG4CXX_STR("Subject"));
}
LogString SMTPAppender::getSMTPHost() const
@@ -697,7 +730,7 @@ LogString SMTPAppender::getTo() const
void SMTPAppender::setTo(const LogString& addressStr)
{
- _priv->to = addressStr;
+ _priv->to = stripSmtpControl(addressStr, LOG4CXX_STR("To"));
}
LogString SMTPAppender::getCc() const
@@ -707,7 +740,7 @@ LogString SMTPAppender::getCc() const
void SMTPAppender::setCc(const LogString& addressStr)
{
- _priv->cc = addressStr;
+ _priv->cc = stripSmtpControl(addressStr, LOG4CXX_STR("Cc"));
}
LogString SMTPAppender::getBcc() const
@@ -717,7 +750,7 @@ LogString SMTPAppender::getBcc() const
void SMTPAppender::setBcc(const LogString& addressStr)
{
- _priv->bcc = addressStr;
+ _priv->bcc = stripSmtpControl(addressStr, LOG4CXX_STR("Bcc"));
}
/**
diff --git a/src/test/cpp/net/smtpappendertestcase.cpp b/src/test/cpp/net/smtpappendertestcase.cpp
index 2b3797cf..5ff10c40 100644
--- a/src/test/cpp/net/smtpappendertestcase.cpp
+++ b/src/test/cpp/net/smtpappendertestcase.cpp
@@ -76,6 +76,9 @@ class SMTPAppenderTestCase : public AppenderSkeletonTestCase
LOGUNIT_TEST(testTrigger);
LOGUNIT_TEST(testInvalid);
LOGUNIT_TEST(testNegativeBufferSizeOption);
+ LOGUNIT_TEST(testSubjectStripsCRLF);
+ LOGUNIT_TEST(testAddressFieldsStripCRLF);
+ LOGUNIT_TEST(testCleanFieldsArePreserved);
//#define LOG4CXX_TEST_EMAIL_AND_SMTP_HOST_ARE_IN_ENVIRONMENT_VARIABLES
#ifdef LOG4CXX_TEST_EMAIL_AND_SMTP_HOST_ARE_IN_ENVIRONMENT_VARIABLES
// This test requires the following environment variables:
@@ -141,6 +144,74 @@ class SMTPAppenderTestCase : public AppenderSkeletonTestCase
LOGUNIT_ASSERT_EQUAL(1, appender.getBufferSize());
}
+ /**
+ * SMTPSession::toAscii is the library's sanitization step for values
+ * destined for SMTP headers via libesmtp; it rewrites non-ASCII to '?'
+ * but does not touch CR/LF. Before the fix, a Subject containing CRLF
+ * was passed verbatim to smtp_set_header — RFC 5322 §2.1 treats CRLF
+ * as the header-field terminator, so an attacker controlling a
+ * configured Subject (e.g. via property substitution) could inject a
+ * fresh Bcc/Cc header. Each public setter must strip CR (0x0D) and
+ * LF (0x0A) so the boundary is enforced regardless of how the value
+ * reaches the appender.
+ */
+ void testSubjectStripsCRLF()
+ {
+ SMTPAppender appender;
+ appender.setSubject(LOG4CXX_STR("Notification\r\nBcc: attacker@example.invalid"));
+ LOGUNIT_ASSERT_EQUAL(
+ LogString(LOG4CXX_STR("NotificationBcc: attacker@example.invalid")),
+ appender.getSubject());
+
+ appender.setSubject(LOG4CXX_STR("alert\nfollow-up"));
+ LOGUNIT_ASSERT_EQUAL(
+ LogString(LOG4CXX_STR("alertfollow-up")),
+ appender.getSubject());
+
+ appender.setSubject(LOG4CXX_STR("loose\rCR"));
+ LOGUNIT_ASSERT_EQUAL(
+ LogString(LOG4CXX_STR("looseCR")),
+ appender.getSubject());
+ }
+
+ void testAddressFieldsStripCRLF()
+ {
+ SMTPAppender appender;
+ appender.setFrom(LOG4CXX_STR("me@example.invalid\r\nBcc: x@y"));
+ appender.setTo(LOG4CXX_STR("you@example.invalid\nBcc: x@y"));
+ appender.setCc(LOG4CXX_STR("a@b\r,c@d"));
+ appender.setBcc(LOG4CXX_STR("z@example.invalid\n"));
+
+ LOGUNIT_ASSERT_EQUAL(
+ LogString(LOG4CXX_STR("me@example.invalidBcc: x@y")),
+ appender.getFrom());
+ LOGUNIT_ASSERT_EQUAL(
+ LogString(LOG4CXX_STR("you@example.invalidBcc: x@y")),
+ appender.getTo());
+ LOGUNIT_ASSERT_EQUAL(
+ LogString(LOG4CXX_STR("a@b,c@d")),
+ appender.getCc());
+ LOGUNIT_ASSERT_EQUAL(
+ LogString(LOG4CXX_STR("z@example.invalid")),
+ appender.getBcc());
+ }
+
+ void testCleanFieldsArePreserved()
+ {
+ // Whitespace, tabs, commas, and non-ASCII are deliberately untouched
+ // here: only CR/LF are stripped. toAscii continues to handle non-ASCII
+ // at SMTP-message construction time.
+ SMTPAppender appender;
+ appender.setSubject(LOG4CXX_STR(" Spaced subject\t"));
+ appender.setTo(LOG4CXX_STR("a@example.invalid, b@example.invalid"));
+ LOGUNIT_ASSERT_EQUAL(
+ LogString(LOG4CXX_STR(" Spaced subject\t")),
+ appender.getSubject());
+ LOGUNIT_ASSERT_EQUAL(
+ LogString(LOG4CXX_STR("a@example.invalid, b@example.invalid")),
+ appender.getTo());
+ }
+
void testValid()
{
auto status = xml::DOMConfigurator::configure("input/xml/smtpAppenderValid.xml");