False positive for CA2100

[nir-yurman]

Analyzer

Diagnostic ID: CA2100: Review SQL queries for security vulnerabilities

Analyzer source

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

Version: SDK 5.0.100

Describe the bug

A false alarm CA2100 is emitted by security analyzer when command text uses text substitution

Steps To Reproduce

Simply compile the following code:

	public SqlCommand GetCommand(int y)
	{
		const int x = 10;
		SqlCommand cmd = new SqlCommand();
		cmd.Parameters.Add("@y", SqlDbType.Int).Value = y;
		
		cmd.CommandText = @$"
SELECT * FROM T
WHERE 
	X = '{x}' 
AND Y = @y";
			
		return cmd;
	}

Expected behavior

No warning is emitted, the code does not allow for SQL injection

Actual behavior

CA2100 is emitted, the analyzer is unhappy with first line after the 'WHERE' statement. Although this is constant and is not something that provided "externally"

Additional context

To solve the warning you would have to do (and change the query to include @x)

		cmd.Parameters.Add("@x", SqlDbType.Int).Value = 10;

[mavasani]

Ah this might be due to the fact that string interpolation, i.e. $ was not always treated as a non-constant string by the C# compiler, even if all its constituents parts are all constants. I believe this might have changed recently with the recent C# language changes, and this no longer holds. I am not sure if this needs a fix in our DFA framework OR needs the users to move to a newer C# langauge version and/or compiler to get the new interpolation feature. This needs more investigations.

[Youssef1313]

I am not sure if this needs a fix in our DFA framework OR needs the users to move to a newer C# langauge version and/or compiler to get the new interpolation feature.

I believe it only needs moving to a newer compiler version only (should be 16.9 based on the milestone in dotnet/roslyn#49676). No need to upgrade to a newer language version.

I also think we won't be able to reproduce this issue in tests if #4741 gets merged. See #4741 (comment)