AD0001: Analyzer 'Microsoft.NetCore.Analyzers.Security.UseAutoValidateAntiforgeryToken' threw an exception of type 'System.ArgumentNullException' with message 'Value cannot be null.'

[notnotchris]

Analyser

AD0001: Analyzer 'Microsoft.NetCore.Analyzers.Security.UseAutoValidateAntiforgeryToken' threw an exception of type 'System.ArgumentNullException' with message 'Value cannot be null.'

Analyser source

SDK: Built-in CA analysers in .NET 5 SDK or later

Version: SDK 5.0.102

Describe the bug

An ArgumentNullException occurs, causing an AD0001 warning, when building a project that registers an asynchronous authorization filter (implements IAsyncAuthorizationFilter) that derives from an abstract class and all rules are enabled by default (<AnalysisMode>AllEnabledByDefault</AnalysisMode> in the project file).

Steps to Reproduce

1. Create an abstract asynchronous authorization filter.
2. Create a concrete class that derives from the abstract type.
3. Register the concrete filter in AddControllers.
4. Enable all rules by default.
5. Build the solution/project.

Expected behaviour

Either the project builds successfully without warnings, or the relevant code analysis warning is displayed.

Actual behaviour

An AD0001 warning is generated.

Additional context

Adding the latest NuGet package version (5.0.3) of the analysers makes no difference. The AD0001 warning is still generated.

The AD0001 warning does not occur when all rules are not enabled by default (i.e. <AnalysisMode>AllEnabledByDefault</AnalysisMode> is omitted from the project file).

A minimum repro is available here.

[Youssef1313]

@notnotchris Thanks for the detailed repro steps. The issue happens when the filter used doesn't directly contain "OnAuthorizationAsync" method.

The problematic part of the analyzer implementation is the following:

roslyn-analyzers/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Security/UseAutoValidateAntiforgeryToken.cs

Lines 196 to 207 in 3fb8781

	onAuthorizationAsyncMethodSymbols.TryAdd(
	potentialAntiForgeryFilter
	.GetMembers()
	.OfType<IMethodSymbol>()
	.FirstOrDefault(
	s => (s.Name == "OnAuthorizationAsync" &&
	s.ReturnType.Equals(taskTypeSymbol) ||
	s.Name == "OnAuthorization" &&
	s.ReturnsVoid) &&
	s.Parameters.Length == 1 &&
	s.Parameters[0].Type.Equals(authorizationFilterContextTypeSymbol)),
	placeholder);

The call to FirstOrDefault will return null, which is passed as the key to onAuthorizationAsyncMethodSymbols.TryAdd

Note: The fix should consider a scenario where the abstract filter has a virtual implementation of OnAuthorizationAsync, which is overriden by the concrete filter. In that case, both potentialAntiForgeryFilter and its base type have OnAuthorizationAsync method symbol. Does it matter which one is added to onAuthorizationAsyncMethodSymbols?

[mavasani]

Tagging @dotpaul

[dotpaul]

Thanks @notnotchris and @Youssef1313! I will work on a fix.

[notnotchris]

@dotpaul when will your fix be released? Is it available in the latest preview release?