Skip to content

Commit 4e08a41

Browse files
[IAST] Change filtered cookie vuln hash (DataDog#6032)
## Summary of changes Update [RFC](https://docs.google.com/document/d/1LLilcVkA1godL5sXuEoMlPFTT-hEF-8f-seGnx_CIvc/edit) changes in filtered cookie calculation ## Reason for change Previous option could collide with a cookie named `Filtered` ## Implementation details Changed the hash calculation to `FILTERED_VULN` instead of `VULN:Filtered` ## Test coverage Added unit test ## Other details <!-- Fixes #{issue} --> <!-- ⚠️ Note: where possible, please obtain 2 approvals prior to merging. Unless CODEOWNERS specifies otherwise, for external teams it is typically best to have one review from a team member, and one review from apm-dotnet. Trivial changes do not require 2 reviews. -->
1 parent c1a6733 commit 4e08a41

File tree

3 files changed

+22
-10
lines changed

3 files changed

+22
-10
lines changed

tracer/src/Datadog.Trace/Iast/IastModule.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ private static string BuildCommandInjectionEvidence(string file, string argument
375375
[MethodImpl(MethodImplOptions.AggressiveInlining)]
376376
internal static int GetCookieHash(string vulnerability, string cookieName, bool isFiltered)
377377
{
378-
return (vulnerability.ToString() + ":" + (isFiltered ? "Filtered" : cookieName)).GetStaticHashCode();
378+
return (isFiltered ? ("FILTERED_" + vulnerability) : (vulnerability + ":" + cookieName)).GetStaticHashCode();
379379
}
380380

381381
public static IastModuleResponse OnInsecureCookie(IntegrationId integrationId, string cookieName, bool isFiltered)

tracer/test/Datadog.Trace.Security.Unit.Tests/IAST/HashTests.cs

+12
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,16 @@ public void GivenAKownVulnerability_WhenCalculatedHash_ValueIsExpected(string vu
6464
var hashCode = vulnerability.GetHashCode();
6565
Assert.Equal(expectedHash, hashCode);
6666
}
67+
68+
[Theory]
69+
[InlineData(VulnerabilityTypeName.InsecureCookie, "AspNetCoreRateLimit.RateLimitProcessor", false, -304624042)]
70+
[InlineData(VulnerabilityTypeName.InsecureCookie, "AspNetCoreRateLimit.RateLimitProcessor", true, 990913114)]
71+
[InlineData(VulnerabilityTypeName.InsecureCookie, "AspNetCore.Views_Iast_ReflectedXss+<<ExecuteAsync>b__8_1>d", true, 990913114)]
72+
[InlineData(VulnerabilityTypeName.NoSameSiteCookie, "AspNetCore.Views_Iast_ReflectedXss+<<ExecuteAsync>b__8_1>d", false, 1003850134)]
73+
[InlineData(VulnerabilityTypeName.NoSameSiteCookie, "AspNetCore.Views_Iast_ReflectedXss+<<ExecuteAsync>b__8_1>d", true, -636226626)]
74+
[InlineData(VulnerabilityTypeName.NoSameSiteCookie, "AspNetCoreRateLimit.RateLimitProcessor", true, -636226626)]
75+
public void GivenACookie_WhenCalculatedHash_ValueIsExpected(string vulnName, string cookieName, bool isFiltered, int hash)
76+
{
77+
IastModule.GetCookieHash(vulnName, cookieName, isFiltered).Should().Be(hash);
78+
}
6779
}

tracer/test/snapshots/Iast.AspNetCore5.enableIast=True.path =_Iast_AllVulnerabilitiesCookie.verified.txt

+9-9
Original file line numberDiff line numberDiff line change
@@ -47,63 +47,63 @@
4747
},
4848
{
4949
"type": "NO_SAMESITE_COOKIE",
50-
"hash": -1837181716,
50+
"hash": -636226626,
5151
"evidence": {
5252
"value": "LongCookie.abcdefghijklmnopqrstuvwxyz0123456789.0"
5353
}
5454
},
5555
{
5656
"type": "NO_HTTPONLY_COOKIE",
57-
"hash": 1990393425,
57+
"hash": -60481650,
5858
"evidence": {
5959
"value": "LongCookie.abcdefghijklmnopqrstuvwxyz0123456789.0"
6060
}
6161
},
6262
{
6363
"type": "INSECURE_COOKIE",
64-
"hash": 1170867602,
64+
"hash": 990913114,
6565
"evidence": {
6666
"value": "LongCookie.abcdefghijklmnopqrstuvwxyz0123456789.0"
6767
}
6868
},
6969
{
7070
"type": "NO_SAMESITE_COOKIE",
71-
"hash": -1837181716,
71+
"hash": -636226626,
7272
"evidence": {
7373
"value": "LongCookie.abcdefghijklmnopqrstuvwxyz0123456789.1"
7474
}
7575
},
7676
{
7777
"type": "NO_HTTPONLY_COOKIE",
78-
"hash": 1990393425,
78+
"hash": -60481650,
7979
"evidence": {
8080
"value": "LongCookie.abcdefghijklmnopqrstuvwxyz0123456789.1"
8181
}
8282
},
8383
{
8484
"type": "INSECURE_COOKIE",
85-
"hash": 1170867602,
85+
"hash": 990913114,
8686
"evidence": {
8787
"value": "LongCookie.abcdefghijklmnopqrstuvwxyz0123456789.1"
8888
}
8989
},
9090
{
9191
"type": "NO_SAMESITE_COOKIE",
92-
"hash": -1837181716,
92+
"hash": -636226626,
9393
"evidence": {
9494
"value": "LongCookie.abcdefghijklmnopqrstuvwxyz0123456789.2"
9595
}
9696
},
9797
{
9898
"type": "NO_HTTPONLY_COOKIE",
99-
"hash": 1990393425,
99+
"hash": -60481650,
100100
"evidence": {
101101
"value": "LongCookie.abcdefghijklmnopqrstuvwxyz0123456789.2"
102102
}
103103
},
104104
{
105105
"type": "INSECURE_COOKIE",
106-
"hash": 1170867602,
106+
"hash": 990913114,
107107
"evidence": {
108108
"value": "LongCookie.abcdefghijklmnopqrstuvwxyz0123456789.2"
109109
}

0 commit comments

Comments
 (0)