escape NUL characters in EventLogAppender content #315

ReportEventW takes a null terminated string, so a NUL in logged content ended the
stored record there and dropped whatever the layout rendered after it. WriteEntry
raises nothing, so the record simply stored short and no ErrorHandler call fired.

Measured on Windows 11 build 26200: a 45 character message with a NUL at 23 stored
as its 23 character prefix.

Escaping happens before the size limit is applied, since it doubles each NUL, and
PrepareEventText exists so that ordering can be tested without an event log.

Author: FreeAndNil

src/changelog/3.5.0/315-eventlog-nul.xml

@@ -0,0 +1,15 @@
+<?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="315" link="https://github.com/apache/logging-log4net/pull/315"/>
+  <description format="asciidoc">
+    escape NUL characters in `EventLogAppender` content. `ReportEventW` takes a null terminated
+ string, so a NUL in logged content ended the stored record there and silently dropped whatever
+ the layout rendered after it, exception text and trailing fields included (CWE-158). `WriteEntry`
+ raises nothing, so the record simply stored short. Measured on Windows 11 build 26200: of a 45
+ character message with a NUL at 23, the 23 character prefix was stored and the rest was gone
+ (audit da18b6fd-f007)
+  </description>
+</entry>

src/log4net.Tests/Appender/EventLogAppenderTest.cs

@@ -21,6 +21,7 @@
 #if NET462_OR_GREATER
 
 using System.Diagnostics;
+using System.Reflection;
 
 using log4net.Appender;
 using log4net.Core;
@@ -81,6 +82,40 @@ public void ActivateOptionsDisablesAppenderIfSourceDoesntExist()
     eventAppender.ActivateOptions();
     Assert.That(eventAppender.Threshold, Is.EqualTo(Level.Off));
   }
+
+  /// <summary>
+  /// ReportEventW takes a null terminated string, so a NUL in content ends the stored record
+  /// there and silently drops whatever the layout rendered after it. Measured on Windows 11
+  /// 26200: WriteEntry does not throw, and only the prefix is stored.
+  /// </summary>
+  [Test]
+  public void NulCharactersAreEscaped()
+    => Assert.That(PrepareEventText("before\0after", 100), Is.EqualTo("before\\0after"));
+
+  /// <summary>
+  /// The escape doubles each NUL, so it has to happen before the limit is applied. Escaping
+  /// afterwards would push a message near the limit back over it.
+  /// </summary>
+  [Test]
+  public void EscapingHappensBeforeTheLimitIsApplied()
+  {
+    const int maxSize = 4;
+
+    string prepared = PrepareEventText("\0\0\0", maxSize);
+
+    Assert.That(prepared, Has.Length.EqualTo(maxSize));
+    Assert.That(prepared, Does.Not.Contain("\0"));
+  }
+
+  /// <summary>A message within the limit and without a NUL comes through untouched.</summary>
+  [Test]
+  public void MessagesWithinTheLimitAreUnchanged()
+    => Assert.That(PrepareEventText("field=1\tfield=2", 100), Is.EqualTo("field=1\tfield=2"));
+
+  private static string PrepareEventText(string rendered, int maxSize)
+    => (string)typeof(EventLogAppender)
+      .GetMethod("PrepareEventText", BindingFlags.Static | BindingFlags.NonPublic)!
+      .Invoke(null, [rendered, maxSize])!;
 }
 
-#endif // NET462_OR_GREATER
+#endif // NET462_OR_GREATER

src/log4net/Appender/EventLogAppender.cs

@@ -23,6 +23,7 @@
 using System.Diagnostics;
 
 using log4net.Util;
+using log4net.Appender.Internal;
 using log4net.Core;
 
 namespace log4net.Appender;
@@ -377,13 +378,7 @@ protected override void Append(LoggingEvent loggingEvent)
     // Write to the event log
     try
     {
-      string eventTxt = RenderLoggingEvent(loggingEvent);
-
-      // There is a limit of about 32K characters for an event log message
-      if (eventTxt.Length > _maxEventlogMessageSize)
-      {
-        eventTxt = eventTxt.Substring(0, _maxEventlogMessageSize);
-      }
+      string eventTxt = PrepareEventText(RenderLoggingEvent(loggingEvent), _maxEventlogMessageSize);
 
       EventLogEntryType entryType = GetEntryType(loggingEvent.Level);
 
@@ -398,6 +393,25 @@ protected override void Append(LoggingEvent loggingEvent)
     }
   }
 
+  /// <summary>
+  /// Escapes the NUL characters and then applies the message size limit.
+  /// </summary>
+  /// <param name="rendered">The rendered event.</param>
+  /// <param name="maxSize">The largest message the event log accepts.</param>
+  /// <returns>The text to write.</returns>
+  /// <remarks>
+  /// <para>
+  /// The order matters. <c>ReportEventW</c> takes a null terminated string, so a NUL in content
+  /// ends the stored record there, and escaping doubles each NUL, so escaping after the limit was
+  /// applied could push the message back over it.
+  /// </para>
+  /// </remarks>
+  private static string PrepareEventText(string rendered, int maxSize)
+  {
+    string escaped = NativeStringEscape.EscapeNulCharacters(rendered);
+    return escaped.Length > maxSize ? escaped.Substring(0, maxSize) : escaped;
+  }
+
   /// <summary>
   /// This appender requires a <see cref="Layout"/> to be set.
   /// </summary>