Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Missing length check when parsing actort claim to JsonWebToken #3075

Open
msbw2 opened this issue Jan 4, 2025 · 2 comments
Open

[Bug] Missing length check when parsing actort claim to JsonWebToken #3075

msbw2 opened this issue Jan 4, 2025 · 2 comments
Labels
Bug Product is not functioning as expected P2 High, but not urgent. Needs to be addressed within the next couple of sprints

Comments

@msbw2
Copy link
Contributor

msbw2 commented Jan 4, 2025

In JsonWebTokenHandler.ValidateJwsAsync the actort claim is parsed into a JsonWebToken using JsonWebTokenHandler.ReadToken without checking the length of the token:

if (validationParameters.ValidateActor && !string.IsNullOrWhiteSpace(jsonWebToken.Actor))
{
ValidationResult<SecurityToken> actorReadingResult = ReadToken(jsonWebToken.Actor, callContext);

jsonWebToken.Actor uses this getter:

/// <summary>
/// Gets the 'value' of the 'actort' claim the payload.
/// </summary>
/// <remarks>
/// If the 'actort' claim is not found, an empty string is returned.
/// </remarks>
public string Actor
{
get
{
_act ??= Payload.GetStringValue(JwtRegisteredClaimNames.Actort);
return _act;
}
}

JsonWebTokenHandler.CanReadToken should be used to check if the actort token can be read before calling JsonWebTokenHandler.ReadToken to ensure the length check among other checks is satisfied. JsonWebTokenHandler.CanReadToken for reference:

/// <summary>
/// Determines if the string is a well formed JSON Web Token (JWT). See: <see href="https://datatracker.ietf.org/doc/html/rfc7519"/>.
/// </summary>
/// <param name="token">String that should represent a valid JWT.</param>
/// <remarks>Uses <see cref="Regex.IsMatch(string, string)"/> matching:
/// <para>JWS: @"^[A-Za-z0-9-_]+\.[A-Za-z0-9-_]+\.[A-Za-z0-9-_]*$"</para>
/// <para>JWE: (dir): @"^[A-Za-z0-9-_]+\.\.[A-Za-z0-9-_]+\.[A-Za-z0-9-_]+\.[A-Za-z0-9-_]*$"</para>
/// <para>JWE: (wrappedkey): @"^[A-Za-z0-9-_]+\.[A-Za-z0-9-_]+\.[A-Za-z0-9-_]+\.[A-Za-z0-9-_]+\.[A-Za-z0-9-_]$"</para>
/// </remarks>
/// <returns>
/// <para><see langword="false"/> if the token is null or whitespace.</para>
/// <para><see langword="false"/> if token.Length is greater than <see cref="TokenHandler.MaximumTokenSizeInBytes"/>.</para>
/// <para><see langword="true"/> if the token is in JSON Compact Serialization format.</para>
/// </returns>
public virtual bool CanReadToken(string token)
{
if (string.IsNullOrWhiteSpace(token))
return false;
if (token.Length > MaximumTokenSizeInBytes)
{
if (LogHelper.IsEnabled(EventLogLevel.Informational))
LogHelper.LogInformation(TokenLogMessages.IDX10209, LogHelper.MarkAsNonPII(token.Length), LogHelper.MarkAsNonPII(MaximumTokenSizeInBytes));
return false;
}
// Count the number of segments, which is the number of periods + 1. We can stop when we've encountered
// more segments than the maximum we know how to handle.
int pos = 0;
int segmentCount = 1;
while (segmentCount <= JwtConstants.MaxJwtSegmentCount && ((pos = token.IndexOf('.', pos)) >= 0))
{
pos++;
segmentCount++;
}
switch (segmentCount)
{
case JwtConstants.JwsSegmentCount:
return JwtTokenUtilities.RegexJws.IsMatch(token);
case JwtConstants.JweSegmentCount:
return JwtTokenUtilities.RegexJwe.IsMatch(token);
default:
LogHelper.LogInformation(LogMessages.IDX14107);
return false;
}
}

All other usages of JsonWebTokenHandler.ReadToken should be checked to ensure a call to JsonWebTokenHandler.CanReadToken is present before each usage.

@msbw2
Copy link
Contributor Author

msbw2 commented Jan 5, 2025

@jennyf19 This bug also impacts SAL in CdtValidator.ReadToken.

@jennyf19 jennyf19 added P2 High, but not urgent. Needs to be addressed within the next couple of sprints and removed needs attention untriaged labels Jan 8, 2025
@jennyf19
Copy link
Collaborator

jennyf19 commented Jan 8, 2025

Thanks @msbw2

@pmaytak pmaytak added the Bug Product is not functioning as expected label Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Product is not functioning as expected P2 High, but not urgent. Needs to be addressed within the next couple of sprints
Projects
None yet
Development

No branches or pull requests

3 participants