Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .oss-scanner/threat_model.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ Matters most:
- `log4net.Appender.Internal.ContentEscape` and `RemoteSyslogAppender.AppendMessage`: the shared
escapes are `\0` for NUL, `\r` and `\n`, and `\uXXXX` for anything else. A sink that cannot carry a
character escapes it visibly; it never drops the character, the rest of the record, or the event.
`AppenderSkeleton.RenderLoggingEvent` escapes lone surrogates for every appender, so a sink needs no
call of its own for them. NUL stays per sink.
- Anything where untrusted content reaches pattern converters, format expansion, or the appenders
that send it on: `UdpAppender`, `RemoteSyslogAppender`, `TelnetAppender`, and the mail appenders,
`SmtpAppender` and `SmtpPickupDirAppender` in core and `SmtpAppender` in `log4net.Ext.Mail`.
Expand Down
3 changes: 3 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,9 @@ What that leaves for this file is where the answers live in the code:
`RemoteSyslogAppender.AppendMessage`. Put new ones in `ContentEscape` rather than in the
appender: four appenders have needed the same two so far. Escaping before a length limit, not
after, since an escape is longer than what it replaces.
- **Lone surrogates are the exception to escaping per sink.** `AppenderSkeleton.RenderLoggingEvent`
escapes them for every appender and has no opt-out, so a sink adds no call of its own. Only text
rendered around it needs one, such as the category of `TraceAppender`. NUL stays per sink.
- Deliberate secure-default choices belong in the changelog with their opt-out named, so that an
upgrade surprise is searchable. See the entries for `SendTimeoutMillis`, `MatchTimeoutMillis` and
`LockTimeoutMillis`.
2 changes: 1 addition & 1 deletion src/Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
<SystemConfigurationConfigurationManagerPackageVersion>4.5.0</SystemConfigurationConfigurationManagerPackageVersion>
<MicrosoftSourceLinkGitHubPackageVersion>8.0.0</MicrosoftSourceLinkGitHubPackageVersion>
<!-- Analyzer packages -->
<MicrosoftNetAnalyzersPackageVersion>8.0.0</MicrosoftNetAnalyzersPackageVersion>
<MicrosoftNetAnalyzersPackageVersion>10.0.114</MicrosoftNetAnalyzersPackageVersion>
<!-- Test Packages -->
<MicrosoftNetTestSdkPackageVersion>17.11.1</MicrosoftNetTestSdkPackageVersion>
<NUnitAnalyzersPackageVersion>4.3.0</NUnitAnalyzersPackageVersion>
Expand Down
10 changes: 10 additions & 0 deletions src/changelog/3.5.1/343-trace-debug-nul.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0" encoding="UTF-8"?>
<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://logging.apache.org/xml/ns"
xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
type="fixed">
<issue id="343" link="https://github.com/apache/logging-log4net/issues/343"/>
<description format="asciidoc">Escape NUL characters as `\0` in the message and category of
`TraceAppender` and `DebugAppender`. The default listener ends in a native call that stops at the
first NUL, which cut the rest of the record off (fixed by @FreeAndNil)</description>
</entry>
16 changes: 16 additions & 0 deletions src/changelog/3.5.1/344-lone-surrogates-escaped-centrally.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?xml version="1.0" encoding="UTF-8"?>
<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://logging.apache.org/xml/ns"
xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
type="fixed">
<issue id="344" link="https://github.com/apache/logging-log4net/issues/344"/>
<description format="asciidoc">Escape lone surrogates as `\uXXXX` in
`AppenderSkeleton.RenderLoggingEvent`, for every appender that uses it, third-party ones included,
and in the categories of `TraceAppender`, `DebugAppender` and `AspNetTraceAppender`.
One such character stopped an appender over a `StreamWriter` with the default encoding until the
process restarted. Output changes where a lone surrogate used to become `?`, U+FFFD or pass through
unchanged, and there is no option to turn it off. The XML layouts keep masking it as `?`. Layouts
now receive a wrapping writer, and `WriteLine()` follows the `NewLine` of the application's writer.
`TraceAppender` and `DebugAppender` also report a failed flush instead of throwing (fixed by
@FreeAndNil)</description>
</entry>
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?xml version="1.0" encoding="UTF-8"?>
<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://logging.apache.org/xml/ns"
xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
type="fixed">
<issue id="344" link="https://github.com/apache/logging-log4net/issues/344"/>
<description format="asciidoc">Keep surrogate pairs whole when a maximum width cuts a pattern
conversion. The result can be one character shorter than the width (fixed by @FreeAndNil)</description>
</entry>
11 changes: 11 additions & 0 deletions src/changelog/3.5.1/345-adonet-retry-cap-after-accepted-insert.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?xml version="1.0" encoding="UTF-8"?>
<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://logging.apache.org/xml/ns"
xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
type="fixed">
<issue id="345" link="https://github.com/apache/logging-log4net/issues/345"/>
<description format="asciidoc">Give up retrying a failed `AdoNetAppender` batch only while the
database has not accepted an insert. Five rejected events in a row discarded the rest of the batch.
A database that fails after start-up costs one round trip per event again (fixed by
@FreeAndNil)</description>
</entry>
11 changes: 11 additions & 0 deletions src/changelog/3.5.1/346-adonet-escape-and-cut.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?xml version="1.0" encoding="UTF-8"?>
<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://logging.apache.org/xml/ns"
xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
type="fixed">
<issue id="346" link="https://github.com/apache/logging-log4net/issues/346"/>
<description format="asciidoc">Escape NUL characters and lone surrogates in `AdoNetAppender` text
parameters, and cut them to `Size` without splitting a surrogate pair. PostgreSQL rejected such
values, which lost the event. A database that can store them now receives `\0` and `\uXXXX` instead
(fixed by @FreeAndNil)</description>
</entry>
10 changes: 10 additions & 0 deletions src/changelog/3.5.1/347-telnet-accepts-do-not-nest.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0" encoding="UTF-8"?>
<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://logging.apache.org/xml/ns"
xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
type="fixed">
<issue id="347" link="https://github.com/apache/logging-log4net/issues/347"/>
<description format="asciidoc">`TelnetAppender` handles each accepted connection on a fresh stack. Since 3.0.0
a connection already waiting was handled inside the callback of the one before it, so clients
that kept reconnecting nested callbacks on one thread (fixed by @FreeAndNil)</description>
</entry>
10 changes: 10 additions & 0 deletions src/changelog/3.5.1/348-local-syslog-max-record-size.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0" encoding="UTF-8"?>
<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://logging.apache.org/xml/ns"
xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
type="fixed">
<issue id="348" link="https://github.com/apache/logging-log4net/issues/348"/>
<description format="asciidoc">`LocalSyslogAppender` cuts a record longer than `MaxRecordSize` (default 204800 UTF-8
bytes), marks it `...[truncated]` and reports it. glibc used to drop a record larger than its
socket send buffer silently (fixed by @FreeAndNil)</description>
</entry>
11 changes: 11 additions & 0 deletions src/changelog/3.5.1/349-mail-max-body-size.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?xml version="1.0" encoding="UTF-8"?>
<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://logging.apache.org/xml/ns"
xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
type="fixed">
<issue id="349" link="https://github.com/apache/logging-log4net/issues/349"/>
<description format="asciidoc">Both `SmtpAppender` implementations split a batch whose body exceeds `MaxBodySize`
(default 1048576 bytes) into several mails and cut a record too long for one. A server used to
refuse the whole batch. The legacy appender sends each part with its own `SendTimeoutMillis`
(fixed by @FreeAndNil)</description>
</entry>
10 changes: 10 additions & 0 deletions src/changelog/3.5.1/350-cdata-end-ordinal-search.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0" encoding="UTF-8"?>
<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://logging.apache.org/xml/ns"
xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
type="fixed">
<issue id="350" link="https://github.com/apache/logging-log4net/issues/350"/>
<description format="asciidoc">Search for the CDATA end marker ordinally in `XmlLayout` and
`XmlLayoutSchemaLog4J`. Text with ignorable characters, or a combining mark after the marker, now
stays intact in its event in every culture (ANT-2026-2PJSIJWL, fixed by @FreeAndNil)</description>
</entry>
12 changes: 12 additions & 0 deletions src/changelog/3.5.1/351-escape-unencodable-characters.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0" encoding="UTF-8"?>
<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://logging.apache.org/xml/ns"
xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
type="fixed">
<issue id="351" link="https://github.com/apache/logging-log4net/issues/351"/>
<description format="asciidoc">Write characters the encoding cannot represent as `\uXXXX`
in `FileAppender`, `RollingFileAppender`, `UdpAppender` and both `SmtpAppender` classes. They used
to become `?`, U+FFFD or a look-alike character. On .NET Framework the default encoding is the
ANSI code page, so text outside it, such as Cyrillic or Chinese, now shows as escapes unless you
set `utf-8` (fixed by @FreeAndNil)</description>
</entry>
11 changes: 11 additions & 0 deletions src/changelog/3.5.1/352-mail-body-transfer-encoding.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?xml version="1.0" encoding="UTF-8"?>
<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://logging.apache.org/xml/ns"
xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
type="fixed">
<issue id="352" link="https://github.com/apache/logging-log4net/issues/352"/>
<description format="asciidoc">The `SmtpAppender` of `log4net.Ext.Mail` sends the body quoted-printable, or base64
for a UTF-16 or UTF-32 `BodyEncoding`. A carriage return from logged content used to reach the
relay bare, and a relay that enforces RFC 5321 refused the whole batch. A body with non-ASCII text
can be up to three times larger on the wire than as 8bit (fixed by @FreeAndNil)</description>
</entry>
10 changes: 10 additions & 0 deletions src/changelog/3.5.1/353-pickup-lone-carriage-return.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0" encoding="UTF-8"?>
<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://logging.apache.org/xml/ns"
xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
type="fixed">
<issue id="353" link="https://github.com/apache/logging-log4net/issues/353"/>
<description format="asciidoc">`SmtpPickupDirAppender` writes a carriage return from logged content that is not
followed by a line feed as `\r`. A strict mail server such as Sendmail refused a picked up mail
with such a character (fixed by @FreeAndNil)</description>
</entry>
12 changes: 12 additions & 0 deletions src/changelog/3.5.1/354-aspnet-request-unread-body.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0" encoding="UTF-8"?>
<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://logging.apache.org/xml/ns"
xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
type="fixed">
<issue id="354" link="https://github.com/apache/logging-log4net/issues/354"/>
<description format="asciidoc">`%aspnet-request` no longer reads the request body from the client. It used to load
the form under the appender lock, so a client withholding its body held every thread logging to the
appender. Form fields now appear only once the application has read the body in classic or buffered
mode, and a bufferless read no longer turns the whole conversion into `NOT AVAILABLE`. Lookups such as
`+%aspnet-request{REMOTE_ADDR}+` are unchanged (fixed by @FreeAndNil)</description>
</entry>
34 changes: 34 additions & 0 deletions src/log4net.Ext.Mail.Tests/Appender/FakeSmtpTransport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ internal sealed class FakeSmtpTransport : ISmtpTransport
/// </summary>
internal Exception? SendException { get; set; }

/// <summary>How many of the next sends fail, as a server refuses an oversize mail.</summary>
internal int SendsToRefuse { get; set; }

/// <summary>When set, every call that takes a token waits this long, honouring it.</summary>
internal int DelayMillisPerCall { get; set; }

Expand Down Expand Up @@ -115,6 +118,11 @@ public void Send(MimeMessage message, CancellationToken cancellationToken)
{
throw exception;
}
if (SendsToRefuse > 0)
{
SendsToRefuse--;
throw new InvalidOperationException("552 5.3.4 Message size exceeds fixed limit");
}
SentMails.Add(new SentMail(message));
}

Expand Down Expand Up @@ -171,6 +179,14 @@ internal SentMail(MimeMessage message)
TextPart? textPart = message.Body as TextPart;
Body = textPart?.Text ?? string.Empty;
BodyCharset = textPart?.ContentType.Charset;
BodyTransferEncoding = textPart?.ContentTransferEncoding ?? ContentEncoding.Default;

// SMTP line ends, as MailKit writes the message onto the wire.
FormatOptions wire = FormatOptions.Default.Clone();
wire.NewLineFormat = NewLineFormat.Dos;
using System.IO.MemoryStream raw = new();
message.WriteTo(wire, raw);
Raw = raw.ToArray();
}

internal string[] From { get; }
Expand All @@ -196,6 +212,24 @@ internal SentMail(MimeMessage message)

internal string? BodyCharset { get; }

internal ContentEncoding BodyTransferEncoding { get; }

/// <summary>The whole message as written to the wire.</summary>
internal byte[] Raw { get; }

/// <summary>Tells whether the message holds a CR that is not followed by LF, which SMTP forbids.</summary>
internal bool HasBareCarriageReturn()
{
for (int i = 0; i < Raw.Length; i++)
{
if (Raw[i] == '\r' && (i + 1 == Raw.Length || Raw[i + 1] != '\n'))
{
return true;
}
}
return false;
}

private static string[] Addresses(InternetAddressList list)
=> list.Mailboxes.Select(m => m.Address).ToArray();
}
Loading
Loading