Avoid passing null pointer from empty span to CreateSymmetricKey

[xtqqczze]

The existing code has an implicit call to symmetricKeyMaterial.GetPinnableReference(), which returns a null reference for _length == 0.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

[vcsjones]

What exactly is this addressing? Is there a missing test?

[vcsjones]

The code above this should guarantee that the Span is never nullptr

runtime/src/libraries/Microsoft.Bcl.Cryptography/src/System/Security/Cryptography/SP800108HmacCounterKdfImplementationCng.cs

Lines 82 to 87 in d6adddf

	else
	{
	// CNG requires a non-null pointer even when the length is zero.
	symmetricKeyMaterial = [0];
	symmetricKeyMaterialLength = 0;
	}

[xtqqczze]

The code above this should guarantee that the Span is never nullptr

We then call GetPinnableReference (implicitly within the fixed statement) which returns NullRef<T>() when the span length is zero:

runtime/src/libraries/System.Private.CoreLib/src/System/ReadOnlySpan.cs

Lines 289 to 295 in d6adddf

	public ref readonly T GetPinnableReference()
	{
	// Ensure that the native code has just one forward branch that is predicted-not-taken.
	ref T ret = ref Unsafe.NullRef<T>();
	if (_length != 0) ret = ref _reference;
	return ref ret;
	}

[vcsjones]

hen the span length is zero:

The span length is never zero though. On line 85 and 35 the span is set to [0] if it is empty and the length is tracked separately. It is guaranteed to not be empty when it is pinned.

[xtqqczze]

hen the span length is zero:

The span length is never zero though. On line 85 and 35 the span is set to [0] if it is empty and the length is tracked separately. It is guaranteed to not be empty when it is pinned.

Yes you are right, in the [0] case the length is actually 1, it is symmetricKeyMaterialLength that is 0.🤦‍♂️