Dewera / Lunar

A lightweight native DLL mapping library that supports mapping directly from memory
MIT License
584 stars 102 forks source link

Duplicate Key in _moduleCache when calling GetModule(string name) #20

Closed CrispCrew closed 3 years ago

CrispCrew commented 3 years ago

ArgumentException: An item with the same key has already been added. Key: NTDLL.dll

This exception was originally thrown at this call stack: System.ThrowHelper.ThrowAddingDuplicateWithKeyArgumentException(T) System.Collections.Generic.Dictionary<TKey, TValue>.TryInsert(TKey, TValue, System.Collections.Generic.InsertionBehavior) System.Collections.Generic.Dictionary<TKey, TValue>.Add(TKey, TValue) Lunar.Remote.ProcessContext.GetModule(string) in ProcessContext.cs Lunar.Remote.ProcessContext.ResolveForwardedFunction(string) in ProcessContext.cs Lunar.Remote.ProcessContext.GetFunctionAddress(string, string) in ProcessContext.cs Lunar.LibraryMapper.BuildImportAddressTable.AnonymousMethod__13_0(Lunar.PortableExecutable.Structures.ImportDescriptor) in LibraryMapper.cs System.Threading.Tasks.Parallel.PartitionerForEachWorker.AnonymousMethod__1(ref System.Collections.IEnumerator, int, out bool) System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw(System.Exception) ... [Call Stack Truncated]

CrispCrew commented 3 years ago

I modified the function to fix the crash temporarily until input from the developer.

private Module GetModule(string moduleName) { moduleName = ResolveModuleName(moduleName);

        if (_moduleCache.TryGetValue(moduleName, out var module))
        {
            return module;
        }

        module = _loader.GetModule(moduleName);

        if (module is null)
        {
            throw new ApplicationException($"Failed to find the module {moduleName} in the process");
        }

        if (_moduleCache.ContainsKey(moduleName))
        {
            throw new ApplicationException($"Duplicate Module {moduleName} in the Process");
        }

        _moduleCache.Add(moduleName, module);

        return module;
    }
Dewera commented 3 years ago

I've had others report similar issues. It's interesting because I don't see how it's possible for this to even be happening.

The code at the top of the function is already doing a key check on the dictionary (which is case insensitive)

if (_moduleCache.TryGetValue(moduleName, out var module))
{
    return module;
}

I believe the issue is actually occurring because of this line meaning it's a concurrency access oversight as I didn't realise that some modules/processes have ntdll.dll in capitals.

I'll do some testing later and try get a fix deployed but for now you could probably solve this without throwing an exception by swapping the data type of _moduleCache to a ConcurrentDictionary