possible null ref in SignatureHelper

[leeriorio]

I analyzed .net runtime sources with Svace static analyzer and it found an error of DEREF_AFTER_NULL category (situations where first, a pointer is compared to NULL (which indicates that it could have a NULL value), and then it is dereferenced (unconditionally)) here

runtime/src/mono/System.Private.CoreLib/src/System/Reflection/Emit/SignatureHelper.cs

Line 307 in 54527ea

if (tone!.Length != ttwo.Length)

Analyzer says "Value tone, which can have null value due to comparison with null, is dereferenced in member access expression tone!.Length"

Searching code history I found commit 5cd7e97 where there was added nullable-forgiving operator. But I think in this situation, it may be a real error: it seems that in lines below

runtime/src/mono/System.Private.CoreLib/src/System/Reflection/Emit/SignatureHelper.cs

Lines 299 to 305 in 54527ea

	if (tone == null)
	{
	if (ttwo == null)
	continue;
	}
	else if (ttwo == null)
	return false;

in situation tone==null && ttwo!=null construction return false must be added. Because this code construction is similar to

runtime/src/mono/System.Private.CoreLib/src/System/Reflection/Emit/SignatureHelper.cs

Lines 315 to 322 in 54527ea

	if (uone == null)
	{
	if (utwo == null)
	continue;
	return false;
	}
	else if (utwo == null)
	return false;

Firstly, in function CompareOK this case tells that comparison is failed. Secondly, there is potentially null in tone can be dereferenced when ttwo is not null. Isn't it?

---

Found by Linux Verification Center (linuxtesting.org) with SVACE.

[KalleOlaviNiemitalo]

I tried to reproduce the null dereference like this

using System;
using System.Reflection;
using System.Reflection.Emit;
using System.Runtime.CompilerServices;

namespace Runtime110832
{
    class Program
    {
        static void Main()
        { 
            AssemblyBuilder assemblyBuilder = AssemblyBuilder.DefineDynamicAssembly(
                new AssemblyName("DemoAssembly"), 
                AssemblyBuilderAccess.Run);
            ModuleBuilder moduleBuilder = assemblyBuilder.DefineDynamicModule("DemoModule");
            SignatureHelper s1 = SignatureHelper.GetMethodSigHelper(
                moduleBuilder,
                CallingConventions.Standard,
                returnType: typeof(void));
            SignatureHelper s2 = SignatureHelper.GetMethodSigHelper(
                moduleBuilder,
                CallingConventions.Standard,
                returnType: typeof(void));
            s1.AddArgument(
                typeof(int),
                requiredCustomModifiers: null,
                optionalCustomModifiers: new Type[] { typeof(IsLong) });
            s2.AddArgument(
                typeof(int),
                requiredCustomModifiers: null,
                optionalCustomModifiers: null);
            _ = s1.Equals(s2);
            _ = s2.Equals(s1);
        }
    }
}

With .NET 9.0.0 on Windows (thus not using the Mono version of SignatureHelper), it does not throw.

I don't have Mono installed. @leeriorio, can you try this code on Mono?

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-reflection-emit
See info in area-owners.md if you want to be subscribed.