Sanitize logs

.github/workflows/dotnetcore.yml

@@ -33,7 +33,12 @@ jobs:
 
     - name: Checkout repository
       uses: actions/[email protected]
-
+
+    - name: Setup .NET 6.x
+      uses: actions/setup-dotnet@v4
+      with:
+        dotnet-version: '6.x'
+
     - name: Setup .NET SDK from global.json
       uses: actions/setup-dotnet@v4
       with:

src/Microsoft.IdentityModel.Logging/LogHelper.cs

@@ -6,6 +6,7 @@
 using System.Diagnostics.Tracing;
 using System.Globalization;
 using System.Linq;
+using System.Text;
 using Microsoft.IdentityModel.Abstractions;
 
 namespace Microsoft.IdentityModel.Logging
@@ -399,7 +400,7 @@ private static object SanitizeSecurityArtifact(object arg)
             // If it's not a ISafeLogSecurityArtifact then just return the object which will be converted to string.
             // It's possible a raw string will contain a security artifact and be exposed here but the alternative is to scrub all objects
             // which defeats the purpose of the ShowPII flag.
-            return arg;
+            return Sanitize(arg.ToString()); //Sanitizes PII strings when ShowPII is true.
         }
 
         private static string RemovePII(object arg)
@@ -408,7 +409,7 @@ private static string RemovePII(object arg)
                 return ex.ToString();
 
             if (arg is NonPII)
-                return arg.ToString();
+                return Sanitize(arg.ToString()); // Sanitizes non-PII
 
             return string.Format(CultureInfo.InvariantCulture, IdentityModelEventSource.HiddenPIIString, arg?.GetType().ToString() ?? "Null");
         }
@@ -523,5 +524,34 @@ private static LogEntry WriteEntry(EventLogLevel eventLogLevel, Exception innerE
 
             return entry;
         }
+
+        /// <summary>
+        /// Sanitizes a string by encoding potentially harmful characters.
+        /// </summary>
+        /// <param name="input">The input string to sanitize</param>
+        /// <returns>A sanitized string safe for logging</returns>
+        private static string Sanitize(string input)
+        {
+            if (string.IsNullOrEmpty(input))
+                return input;
+
+            var sanitized = new StringBuilder(input.Length);
+
+            foreach (char c in input)
+            {
+                if (c == '\r')
+                    sanitized.Append("\\r");
+                else if (c == '\n')
+                    sanitized.Append("\\n");
+                else if (c == '\t')
+                    sanitized.Append("\\t");
+                else if (char.IsControl(c) || CharUnicodeInfo.GetUnicodeCategory(c) == UnicodeCategory.Format)
+                    sanitized.Append($"\\u{(int)c:X4}");
+                else
+                    sanitized.Append(c);
+            }
+
+            return sanitized.ToString();
+        }
     }
 }

test/Microsoft.IdentityModel.Logging.Tests/LogHelperTests.cs

@@ -437,6 +437,58 @@ public void FormatInvariant_NoArgs_ReturnsFormatString()
             // Assert
             Assert.Equal("This is a string with no arguments.", result);
         }
+
+        [Fact]
+        public void FormatInvariant_NonPIIArgument_SanitizesSpecialCharacters()
+        {
+            string format = "Value: {0}";
+            object[] args = new object[] { LogHelper.MarkAsNonPII("A\rB\nC\tD\r\nE" + (char)2) };
+
+            string result = LogHelper.FormatInvariant(format, args);
+
+            Assert.Equal("Value: A\\rB\\nC\\tD\\r\\nE\\u0002", result);
+        }
+
+        [Fact]
+        public void FormatInvariant_PIIArgument_DoesNotSanitizeWhenShowPIIDisabled()
+        {
+            string format = "Value: {0}";
+            object[] args = new object[] { "A\rB\nC\tD\r\nE" + (char)2 };
+            IdentityModelEventSource.ShowPII = false;
+
+            string result = LogHelper.FormatInvariant(format, args);
+
+            Assert.Equal($"Value: {string.Format(IdentityModelEventSource.HiddenPIIString, typeof(string).ToString())}", result);
+
+            IdentityModelEventSource.ShowPII = false;
+        }
+
+        [Fact]
+        public void FormatInvariant_PIIArgument_SanitizeWhenShowPIIEnabled()
+        {
+            string format = "Value: {0}";
+            object[] args = new object[] { "A\rB\nC\tD\r\nE" + (char)2 };
+            IdentityModelEventSource.ShowPII = true;
+
+            string result = LogHelper.FormatInvariant(format, args);
+
+            Assert.Equal("Value: A\\rB\\nC\\tD\\r\\nE\\u0002", result);
+
+            IdentityModelEventSource.ShowPII = false;
+        }
+
+        [Fact]
+        public void FormatInvariant_NonPIIArgument_SanitizesUnicodeFormatCharacters()
+        {
+            // U+200B ZERO WIDTH SPACE and U+2060 WORD JOINER are Unicode format characters
+            string format = "Value: {0}";
+            string input = "A" + '\u200B' + "B" + '\u2060' + "C";
+
+            string result = LogHelper.FormatInvariant(format, LogHelper.MarkAsNonPII(input));
+
+            // Both format characters should be replaced with their \uXXXX representation
+            Assert.Equal("Value: A\\u200BB\\u2060C", result);
+        }
     }
 
     public class MockSecurityToken : SecurityToken

test/Microsoft.IdentityModel.Logging.Tests/TestLogger.cs

@@ -11,6 +11,12 @@ namespace Microsoft.IdentityModel.Logging.Tests
     public class TestLogger : IIdentityLogger
     {
         readonly List<Tuple<string, EventLogLevel>> _logs = new List<Tuple<string, EventLogLevel>>();
+        bool _saveLogs = true;
+
+        public TestLogger(bool saveLogs = true)
+        {
+            _saveLogs = saveLogs;
+        }
 
         public bool IsLoggerEnabled { get; set; } = true;
 
@@ -21,7 +27,8 @@ public bool IsEnabled(EventLogLevel logLevel)
 
         public void Log(LogEntry entry)
         {
-            _logs.Add(new Tuple<string, EventLogLevel>(entry.Message, entry.EventLogLevel));
+            if (_saveLogs)
+                _logs.Add(new Tuple<string, EventLogLevel>(entry.Message, entry.EventLogLevel));
         }
 
         public bool LogStartsWith(string prefix, EventLogLevel logLevel)