Jwt icon indicating copy to clipboard operation
Jwt copied to clipboard

ECJwk GetCanonicalizeSize wrong - Kid randomly wrong

Open inf9144 opened this issue 10 months ago • 2 comments

Hello, in EC JWK the size is calculated by:

	protected internal override int GetCanonicalizeSize()
	{
		return 35 + Base64Url.GetArraySizeRequiredToEncode(Crv.Name.EncodedUtf8Bytes.Length) + Base64Url.GetArraySizeRequiredToEncode(X.Length) + Base64Url.GetArraySizeRequiredToEncode(Y.Length);
	}

but it should be

	protected internal override int GetCanonicalizeSize()
	{
		return 35 + Crv.Name.EncodedUtf8Bytes.Length + Base64Url.GetArraySizeRequiredToEncode(X.Length) + Base64Url.GetArraySizeRequiredToEncode(Y.Length);
	}

because Crv.Name.EncodedUtf8Bytes is copied in Canonicalize:

	protected internal override void Canonicalize(Span<byte> buffer)
	{
		int length = StartCanonicalizeValue.Length;
		StartCanonicalizeValue.CopyTo(buffer);
		EllipticalCurve ellipticalCurve = Crv;
		ellipticalCurve.Name.EncodedUtf8Bytes.CopyTo(buffer.Slice(length));
		int num = length;
		ellipticalCurve = Crv;
		length = num + ellipticalCurve.Name.EncodedUtf8Bytes.Length;
		Middle1CanonicalizeValue.CopyTo(buffer.Slice(length));
		length += Middle1CanonicalizeValue.Length;
		length += Base64Url.Encode(X, buffer.Slice(length));
		Middle2CanonicalizeValue.CopyTo(buffer.Slice(length));
		length += Middle2CanonicalizeValue.Length;
		length += Base64Url.Encode(Y, buffer.Slice(length));
		EndCanonicalizeValue.CopyTo(buffer.Slice(length));
	}

This is a problem because ComputeThumbprint uses this Value for the call to ComputeHash:

Sha256.Shared.ComputeHash(buffer.Slice(0, canonicalizeSize), span);

The buffer is either from ArrayPool or from Stackalloc - ArrayPool does not garantuee to null the array. So you get random Kids when the last bytes differ because they dont get initialized.

I think the Renting is also wrong - you use Stackalloc if canonicalSize is bigger than 256 - i think the condition should be reversed - using stack alloc for smaller arrays right?

I didnt check for RSA and the others, potentially they share some of those bugs.

I can do PRs if you like. Right now our project is running on your "2.0.0-beta.4" i would like to have a stable version :-)

If you dont do maintenance on this project i would be ready to fork this - your library is awesome and much better than the crap Microsoft implemented for their authorization. Want to see it fly :-)

inf9144 avatar Apr 19 '24 10:04 inf9144