Merge branch 'LogExperts:Development' into feature/dark-theme-color-tuning

Author: mfkeeq199458

src/LogExpert.Core/Classes/JsonConverters/ColumnizerJsonConverter.cs

@@ -96,8 +96,11 @@ public override object ReadJson (JsonReader reader, Type objectType, object? exi
             columnizerType = Type.GetType(typeName, throwOnError: false);
             //FindColumnizerTypeByAssemblyQualifiedName(typeName);
 
-            // If Type.GetType fails, try finding it manually
-            if (columnizerType == null)
+            // If Type.GetType fails or finds something that is not a columnizer, try finding it manually
+            if (columnizerType == null ||
+                !typeof(ILogLineMemoryColumnizer).IsAssignableFrom(columnizerType) ||
+                columnizerType.IsInterface ||
+                columnizerType.IsAbstract)
             {
                 columnizerType = FindColumnizerTypeByAssemblyQualifiedName(typeName);
             }

src/LogExpert.Tests/ColumnizerTests/ColumnizerJsonConverterTests.cs

@@ -105,9 +105,39 @@ public void PushValue (ILogLineMemoryColumnizerCallback callback, int column, st
 
 }
 
+public sealed class NotAColumnizer
+{
+    public static int ConstructorCalls { get; set; }
+
+    public NotAColumnizer ()
+    {
+        ConstructorCalls++;
+    }
+}
+
 [TestFixture]
 public class ColumnizerJsonConverterTests
 {
+    [Test]
+    public void Deserialize_TypeNameIsNotAColumnizer_ThrowsWithoutCreatingInstance ()
+    {
+        NotAColumnizer.ConstructorCalls = 0;
+        var json = JsonConvert.SerializeObject(new
+        {
+            TypeName = typeof(NotAColumnizer).AssemblyQualifiedName,
+            State = new { }
+        });
+
+        var settings = new JsonSerializerSettings
+        {
+            TypeNameHandling = TypeNameHandling.None,
+            Converters = { new ColumnizerJsonConverter() }
+        };
+
+        _ = Assert.Throws<JsonSerializationException>(() => JsonConvert.DeserializeObject<ILogLineMemoryColumnizer>(json, settings));
+        Assert.That(NotAColumnizer.ConstructorCalls, Is.Zero);
+    }
+
     [Test]
     public void SerializeDeserialize_MockColumnizer_RoundTripPreservesStateAndType ()
     {

src/PluginRegistry.Tests/AssemblyResolverTests.cs

@@ -0,0 +1,70 @@
+using System.Reflection;
+using System.Reflection.Emit;
+
+using NUnit.Framework;
+
+namespace LogExpert.PluginRegistry.Tests;
+
+/// <summary>
+/// The assembly resolver is reachable from type names stored in settings and .lxp files,
+/// so it must never load assemblies from outside the application and plugin directories.
+/// </summary>
+[TestFixture]
+[Category("PathTraversal")]
+[Category("Security")]
+public class AssemblyResolverTests
+{
+    private string _probeDirectory = null!;
+    private string _probeAssemblyName = null!;
+
+    [SetUp]
+    public void SetUp ()
+    {
+        var id = Guid.NewGuid().ToString("N");
+        _probeAssemblyName = "ResolveProbe_" + id;
+
+        // Sibling of the base directory, so it is reachable through "../" but outside it.
+        _probeDirectory = Path.GetFullPath(Path.Join(AppDomain.CurrentDomain.BaseDirectory, "..", "resolve-probe-" + id));
+        _ = Directory.CreateDirectory(_probeDirectory);
+
+        var assemblyBuilder = new PersistedAssemblyBuilder(new AssemblyName(_probeAssemblyName), typeof(object).Assembly);
+        var typeBuilder = assemblyBuilder.DefineDynamicModule(_probeAssemblyName).DefineType("Probe", TypeAttributes.Public);
+        _ = typeBuilder.CreateType();
+        assemblyBuilder.Save(Path.Join(_probeDirectory, _probeAssemblyName + ".dll"));
+    }
+
+    [TearDown]
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Unit Tests")]
+    public void TearDown ()
+    {
+        try
+        {
+            Directory.Delete(_probeDirectory, recursive: true);
+        }
+        catch
+        {
+            // The DLL stays locked if a regression loaded it; the assertion already reported that.
+        }
+    }
+
+    [Test]
+    public void ResolveHandler_AssemblyNameWithRelativePath_DoesNotLoadAssemblyOutsideBaseDirectory ()
+    {
+        var traversalName = $"../{Path.GetFileName(_probeDirectory)}/{_probeAssemblyName}";
+
+        // Call the handler directly: the test host installs its own resolvers that would mask the result.
+        var handler = typeof(PluginRegistry).GetMethod("ColumnizerResolveEventHandler", BindingFlags.NonPublic | BindingFlags.Static);
+        var resolved = handler!.Invoke(null, [null, new ResolveEventArgs(traversalName)]);
+
+        Assert.That(resolved, Is.Null);
+    }
+
+    [Test]
+    public void ResolveHandler_PlainAssemblyName_IsStillResolvedFromBaseDirectory ()
+    {
+        var handler = typeof(PluginRegistry).GetMethod("ColumnizerResolveEventHandler", BindingFlags.NonPublic | BindingFlags.Static);
+        var resolved = handler!.Invoke(null, [null, new ResolveEventArgs("LogExpert.PluginRegistry")]) as Assembly;
+
+        Assert.That(resolved?.GetName().Name, Is.EqualTo("LogExpert.PluginRegistry"));
+    }
+}

src/PluginRegistry/PluginRegistry.cs

@@ -1277,7 +1277,17 @@ private static T TryInstantiate<T> (Type loadedType, IFileSystemCallback fsCallb
 
     private static Assembly ColumnizerResolveEventHandler (object? sender, ResolveEventArgs args)
     {
-        var fileName = new AssemblyName(args.Name).Name + ".dll";
+        var assemblyName = new AssemblyName(args.Name).Name;
+
+        // Type names come from settings and .lxp files; a name like "../x" must not reach outside our directories.
+        if (string.IsNullOrEmpty(assemblyName) ||
+            assemblyName != Path.GetFileName(assemblyName) ||
+            assemblyName.IndexOfAny(Path.GetInvalidFileNameChars()) >= 0)
+        {
+            return null;
+        }
+
+        var fileName = assemblyName + ".dll";
 
         var mainDir = Path.Join(AppDomain.CurrentDomain.BaseDirectory, fileName);
         var pluginDir = Path.Join(AppDomain.CurrentDomain.BaseDirectory, "plugins", fileName);