From df0453deb13b8dc6f43593eb68dd5ecd46b9f7a8 Mon Sep 17 00:00:00 2001 From: aaronburtle <93220300+aaronburtle@users.noreply.github.com> Date: Wed, 6 May 2026 14:39:04 -0700 Subject: [PATCH] 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 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 Co-authored-by: Aniruddh Munde (cherry picked from commit 0512b00fef0d38ff7f1b684c7abf92310277c90a) --- .../Authorization/AuthorizationResolver.cs | 7 ++- .../AuthorizationResolverUnitTests.cs | 54 +++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/src/Core/Authorization/AuthorizationResolver.cs b/src/Core/Authorization/AuthorizationResolver.cs index 7c3c2fedb2..205dc3d646 100644 --- a/src/Core/Authorization/AuthorizationResolver.cs +++ b/src/Core/Authorization/AuthorizationResolver.cs @@ -845,7 +845,12 @@ private static string GetClaimValue(Claim claim) switch (claim.ValueType) { case ClaimValueTypes.String: - return $"'{claim.Value}'"; + // Escape embedded single quotes per OData 4.01 ABNF (Section 7: Literal Data Values) + // by doubling them. This prevents an attacker-influenced claim value from breaking + // out of the string literal and injecting additional OData predicates into the + // database authorization policy expression. + // See: http://docs.oasis-open.org/odata/odata/v4.01/cs01/abnf/odata-abnf-construction-rules.txt + return $"'{claim.Value.Replace("'", "''")}'"; case ClaimValueTypes.Boolean: case ClaimValueTypes.Integer: case ClaimValueTypes.Integer32: diff --git a/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs b/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs index c16d362268..d1d3f47c49 100644 --- a/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs +++ b/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs @@ -1155,6 +1155,60 @@ public void ParseValidDbPolicy(string policy, string expectedParsedPolicy) Assert.AreEqual(parsedPolicy, expectedParsedPolicy); } + /// + /// Validates that single quote characters embedded in a string-typed claim value are + /// escaped (doubled) per OData 4.01 ABNF when substituted into a database authorization + /// policy. Without escaping, an attacker who can influence a referenced JWT claim could + /// break out of the string literal and inject additional OData predicates - bypassing + /// row-level authorization. The substituted claim must remain enclosed in a single + /// string literal regardless of its contents. + /// + /// The raw claim value (as it appears in the JWT) to substitute. + /// The parsed policy after safe substitution. + [DataTestMethod] + [DataRow( + "alice' or 1 eq 1 or '", + "col1 eq 'alice'' or 1 eq 1 or '''", + DisplayName = "Injection attempt with OR predicate is neutralized by escaping single quotes")] + [DataRow( + "O'Brien", + "col1 eq 'O''Brien'", + DisplayName = "Legitimate single-quote-bearing value (e.g. surname) is safely escaped")] + [DataRow( + "''", + "col1 eq ''''''", + DisplayName = "Value composed solely of single quotes is fully escaped")] + [DataRow( + "no quotes here", + "col1 eq 'no quotes here'", + DisplayName = "Value without single quotes is unchanged aside from enclosing quotes")] + public void DbPolicy_StringClaim_SingleQuotesEscaped_PreventsODataInjection( + string claimValue, + string expectedParsedPolicy) + { + const string policyDefinition = "@item.col1 eq @claims.userId"; + + RuntimeConfig runtimeConfig = InitRuntimeConfig( + entityName: TEST_ENTITY, + roleName: TEST_ROLE, + operation: TEST_OPERATION, + includedCols: new HashSet { "col1" }, + databasePolicy: policyDefinition); + AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); + + Mock context = new(); + + ClaimsIdentity identity = new(TEST_AUTHENTICATION_TYPE, TEST_CLAIMTYPE_NAME, AuthenticationOptions.ROLE_CLAIM_TYPE); + identity.AddClaim(new Claim("userId", claimValue, ClaimValueTypes.String)); + ClaimsPrincipal principal = new(identity); + context.Setup(x => x.User).Returns(principal); + context.Setup(x => x.Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER]).Returns(TEST_ROLE); + + string parsedPolicy = authZResolver.ProcessDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object); + + Assert.AreEqual(expectedParsedPolicy, parsedPolicy); + } + /// /// Tests authorization policy processing mechanism by validating value type compatibility /// of claims present in HttpContext.User.Claims.