This is an automated email from the ASF dual-hosted git repository. FreeAndNil pushed a commit to branch Feature/security-audit-hardening in repository https://gitbox.apache.org/repos/asf/logging-log4net.git
commit eccb876ea4c07c3f3080835b7141b71077b12321 Author: Jan Friedrich <[email protected]> AuthorDate: Mon Aug 17 22:23:24 2026 +0200 escape NUL characters in LocalSyslogAppender messages The rendered message is marshaled to libc as a null-terminated string, so a NUL character anywhere in it ended the record there and silently dropped everything the layout rendered after it, including trailing fields and exception text. Logged content is not trusted and a NUL in it is an in-scope input, so an attacker who gets one logged could hide the tail of every record. Confirmed with the same marshalling the appender uses: for a 24 character message with a NUL in the middle, libc sees 13 characters. NUL is now escaped as \0. Other control characters are still passed through, because syslog(3) encodes them itself and newlines are needed for the multi-line output an exception layout produces. RemoteSyslogAppender drops unprintable characters instead, which would lose the stack traces this appender is expected to carry. Also switches the single statement tests added for the send timeout to expression bodies. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> --- .../309-escape-nul-in-local-syslog-messages.xml | 14 ++++ .../Appender/LocalSyslogAppenderTest.cs | 78 ++++++++++++++++++++++ src/log4net.Tests/Appender/TelnetAppenderTest.cs | 6 +- src/log4net/Appender/LocalSyslogAppender.cs | 22 +++++- 4 files changed, 114 insertions(+), 6 deletions(-) diff --git a/src/changelog/3.4.0/309-escape-nul-in-local-syslog-messages.xml b/src/changelog/3.4.0/309-escape-nul-in-local-syslog-messages.xml new file mode 100644 index 00000000..1ab3b993 --- /dev/null +++ b/src/changelog/3.4.0/309-escape-nul-in-local-syslog-messages.xml @@ -0,0 +1,14 @@ +<?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="309" link="https://github.com/apache/logging-log4net/pull/309"/> + <description format="asciidoc"> + stop a NUL character in logged content from truncating `LocalSyslogAppender` records. The + message is marshaled to libc as a null-terminated string, so everything the layout rendered after + the NUL was silently dropped, including trailing fields and exception text (CWE-158). NUL is now + escaped as `\0`; other control characters are still passed through, because `syslog(3)` encodes + them and newlines are needed for multi-line exception output (audit 1231d72-f008) + </description> +</entry> diff --git a/src/log4net.Tests/Appender/LocalSyslogAppenderTest.cs b/src/log4net.Tests/Appender/LocalSyslogAppenderTest.cs new file mode 100644 index 00000000..ff8cb8de --- /dev/null +++ b/src/log4net.Tests/Appender/LocalSyslogAppenderTest.cs @@ -0,0 +1,78 @@ +#region Apache License +// +// 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. +// +#endregion + +using System.Reflection; + +using log4net.Appender; + +using NUnit.Framework; + +namespace log4net.Tests.Appender; + +/// <summary> +/// Tests for <see cref="LocalSyslogAppender"/> +/// </summary> +/// <remarks> +/// <para> +/// The appender itself writes through <c>syslog(3)</c>, whose output cannot be read back from a +/// test, so these tests cover the message preparation that happens before the native call. +/// </para> +/// </remarks> +[TestFixture] +public class LocalSyslogAppenderTest +{ + /// <summary> + /// The message is marshaled to libc as a null-terminated string, so a NUL character in logged + /// content would end the record there and drop everything the layout rendered after it. + /// </summary> + [Test] + public void NulCharactersAreEscaped() + => Assert.That(EscapeNulCharacters("priority=high\0user=alice"), Is.EqualTo("priority=high\\0user=alice")); + + /// <summary> + /// Several NUL characters must all be escaped, not just the first one. + /// </summary> + [Test] + public void EveryNulCharacterIsEscaped() + => Assert.That(EscapeNulCharacters("a\0b\0c"), Is.EqualTo("a\\0b\\0c")); + + /// <summary> + /// A message without a NUL character has to come through untouched, including the newlines an + /// exception layout produces: <c>syslog(3)</c> deals with those itself. + /// </summary> + [Test] + public void MessagesWithoutNulCharactersAreUnchanged() + { + const string message = "System.InvalidOperationException: boom\r\n at Program.Main()\tfield=1"; + + Assert.That(EscapeNulCharacters(message), Is.EqualTo(message)); + } + + /// <summary> + /// An empty message takes the fast path, which must not turn it into anything else. + /// </summary> + [Test] + public void EmptyMessageIsUnchanged() + => Assert.That(EscapeNulCharacters(string.Empty), Is.Empty); + + private static string EscapeNulCharacters(string message) + => (string)typeof(LocalSyslogAppender) + .GetMethod("EscapeNulCharacters", BindingFlags.Static | BindingFlags.NonPublic)! + .Invoke(null, [message])!; +} diff --git a/src/log4net.Tests/Appender/TelnetAppenderTest.cs b/src/log4net.Tests/Appender/TelnetAppenderTest.cs index cde2231d..e5c238c3 100644 --- a/src/log4net.Tests/Appender/TelnetAppenderTest.cs +++ b/src/log4net.Tests/Appender/TelnetAppenderTest.cs @@ -135,11 +135,7 @@ void WaitForReceived(string what, string expected) /// </summary> [Test] public void SendTimeoutMillisDefaultsToAFiniteValue() - { - TelnetAppender appender = new(); - - Assert.That(appender.SendTimeoutMillis, Is.EqualTo(5000)); - } + => Assert.That(new TelnetAppender().SendTimeoutMillis, Is.EqualTo(5000)); /// <summary> /// 0 is the documented opt-out that restores blocking indefinitely; a negative timeout has no diff --git a/src/log4net/Appender/LocalSyslogAppender.cs b/src/log4net/Appender/LocalSyslogAppender.cs index df481c46..abdbbcb3 100644 --- a/src/log4net/Appender/LocalSyslogAppender.cs +++ b/src/log4net/Appender/LocalSyslogAppender.cs @@ -331,13 +331,33 @@ public override void ActivateOptions() protected override void Append(LoggingEvent loggingEvent) { int priority = GeneratePriority(Facility, GetSeverity(loggingEvent.EnsureNotNull().Level)); - string message = RenderLoggingEvent(loggingEvent); + string message = EscapeNulCharacters(RenderLoggingEvent(loggingEvent)); // Call the local libc syslog method // The second argument is a printf style format string NativeMethods.syslog(priority, "%s", message); } + /// <summary> + /// Replaces NUL characters with a visible <c>\0</c> escape. + /// </summary> + /// <param name="message">The rendered message.</param> + /// <returns>The message with every NUL character escaped.</returns> + /// <remarks> + /// <para> + /// The message is marshaled to libc as a null-terminated string, so a NUL character anywhere in + /// it would end the record there and silently drop everything the layout rendered after it, + /// including trailing fields and exception text. Logged content is not trusted and may well + /// contain a NUL, so the character is escaped rather than passed through. + /// </para> + /// <para> + /// Other control characters are left alone: <c>syslog(3)</c> encodes them itself, and newlines + /// are needed for the multi-line output an exception layout produces. + /// </para> + /// </remarks> + private static string EscapeNulCharacters(string message) + => message.IndexOf('\0') < 0 ? message : message.Replace("\0", "\\0"); + /// <summary> /// Close the syslog when the appender is closed /// </summary>
