Skip to content

Commit 0512b00

Browse files
aaronburtlesouvikghosh04Aniruddh25
authored
Fix OData filter format in JWT string claims (#3510)
## Why make this change? Fixes the format of the OData filter in JWT string claims. ## What is this change? In `AuthorizationResolver` we now escape embedded single quotes in claim values by doubling them, before we wrap the value in single quotes for OData substitution. This conforms to the OData 4.01 ABNF rule for string literals (Section 7: Literal Data Values). Policy: `@item.col1 eq @claims.userId` Claim `userId` value: `alice' or 1 eq 1 or '` | | Resulting OData predicate | | --- | --- | | Before | `col1 eq 'alice' or 1 eq 1 or ''` <- injects `or 1 eq 1`, bypassing row-level auth | | After | `col1 eq 'alice'' or 1 eq 1 or '''` <- attacker payload contained inside a single string literal | ## How was this tested? New parameterized test `DbPolicy_StringClaim_SingleQuotesEscaped_PreventsODataInjection` in `src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs` covers: - Active OR-predicate injection attempt is neutralized. - Legitimate apostrophe-bearing value (e.g. `O'Brien`) is safely escaped. - Value composed solely of single quotes is fully escaped. - Value with no single quotes is unchanged aside from the enclosing quotes (no regression). ## Sample Request(s) ```json { "entities": { "Note": { "source": "dbo.Notes", "permissions": [ { "role": "authenticated", "actions": [ { "action": "read", "policy": { "database": "@item.ownerId eq @claims.userId" } } ] } ] } } } ``` Reproduction - `userId` claim value of `alice' or 1 eq 1 or '`: ```http GET /api/Note HTTP/1.1 Authorization: Bearer <jwt-with-crafted-userId-claim> X-MS-API-ROLE: authenticated ``` - Before fix: the engine emitted `WHERE ownerId = 'alice' or 1 eq 1 or ''`, returning rows owned by other users. - After fix: the engine emits `WHERE ownerId = 'alice'' or 1 eq 1 or '''`, which compares against the literal string `alice' or 1 eq 1 or '` and returns no unauthorized rows. Co-authored-by: Souvik Ghosh <souvikofficial04@gmail.com> Co-authored-by: Aniruddh Munde <anmunde@microsoft.com>
1 parent b2cec31 commit 0512b00

2 files changed

Lines changed: 60 additions & 1 deletion

File tree

src/Core/Authorization/AuthorizationResolver.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -845,7 +845,12 @@ private static string GetClaimValue(Claim claim)
845845
switch (claim.ValueType)
846846
{
847847
case ClaimValueTypes.String:
848-
return $"'{claim.Value}'";
848+
// Escape embedded single quotes per OData 4.01 ABNF (Section 7: Literal Data Values)
849+
// by doubling them. This prevents an attacker-influenced claim value from breaking
850+
// out of the string literal and injecting additional OData predicates into the
851+
// database authorization policy expression.
852+
// See: http://docs.oasis-open.org/odata/odata/v4.01/cs01/abnf/odata-abnf-construction-rules.txt
853+
return $"'{claim.Value.Replace("'", "''")}'";
849854
case ClaimValueTypes.Boolean:
850855
case ClaimValueTypes.Integer:
851856
case ClaimValueTypes.Integer32:

src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1155,6 +1155,60 @@ public void ParseValidDbPolicy(string policy, string expectedParsedPolicy)
11551155
Assert.AreEqual(parsedPolicy, expectedParsedPolicy);
11561156
}
11571157

1158+
/// <summary>
1159+
/// Validates that single quote characters embedded in a string-typed claim value are
1160+
/// escaped (doubled) per OData 4.01 ABNF when substituted into a database authorization
1161+
/// policy. Without escaping, an attacker who can influence a referenced JWT claim could
1162+
/// break out of the string literal and inject additional OData predicates - bypassing
1163+
/// row-level authorization. The substituted claim must remain enclosed in a single
1164+
/// string literal regardless of its contents.
1165+
/// </summary>
1166+
/// <param name="claimValue">The raw claim value (as it appears in the JWT) to substitute.</param>
1167+
/// <param name="expectedParsedPolicy">The parsed policy after safe substitution.</param>
1168+
[DataTestMethod]
1169+
[DataRow(
1170+
"alice' or 1 eq 1 or '",
1171+
"col1 eq 'alice'' or 1 eq 1 or '''",
1172+
DisplayName = "Injection attempt with OR predicate is neutralized by escaping single quotes")]
1173+
[DataRow(
1174+
"O'Brien",
1175+
"col1 eq 'O''Brien'",
1176+
DisplayName = "Legitimate single-quote-bearing value (e.g. surname) is safely escaped")]
1177+
[DataRow(
1178+
"''",
1179+
"col1 eq ''''''",
1180+
DisplayName = "Value composed solely of single quotes is fully escaped")]
1181+
[DataRow(
1182+
"no quotes here",
1183+
"col1 eq 'no quotes here'",
1184+
DisplayName = "Value without single quotes is unchanged aside from enclosing quotes")]
1185+
public void DbPolicy_StringClaim_SingleQuotesEscaped_PreventsODataInjection(
1186+
string claimValue,
1187+
string expectedParsedPolicy)
1188+
{
1189+
const string policyDefinition = "@item.col1 eq @claims.userId";
1190+
1191+
RuntimeConfig runtimeConfig = InitRuntimeConfig(
1192+
entityName: TEST_ENTITY,
1193+
roleName: TEST_ROLE,
1194+
operation: TEST_OPERATION,
1195+
includedCols: new HashSet<string> { "col1" },
1196+
databasePolicy: policyDefinition);
1197+
AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig);
1198+
1199+
Mock<HttpContext> context = new();
1200+
1201+
ClaimsIdentity identity = new(TEST_AUTHENTICATION_TYPE, TEST_CLAIMTYPE_NAME, AuthenticationOptions.ROLE_CLAIM_TYPE);
1202+
identity.AddClaim(new Claim("userId", claimValue, ClaimValueTypes.String));
1203+
ClaimsPrincipal principal = new(identity);
1204+
context.Setup(x => x.User).Returns(principal);
1205+
context.Setup(x => x.Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER]).Returns(TEST_ROLE);
1206+
1207+
string parsedPolicy = authZResolver.ProcessDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object);
1208+
1209+
Assert.AreEqual(expectedParsedPolicy, parsedPolicy);
1210+
}
1211+
11581212
/// <summary>
11591213
/// Tests authorization policy processing mechanism by validating value type compatibility
11601214
/// of claims present in HttpContext.User.Claims.

0 commit comments

Comments
 (0)