[ASM] RASP: Command injection vulnerability implementation #6323

NachoEchevarria · 2024-11-20T09:35:55Z

Summary of changes

This PR contains the implementation of the RASP command injection vulnerability. Previously, the shell injection vulnerability was implemented, which is similar but only occurs the flag UseShellExecute is set to false in method process.start().

The definition of this new vulnerability can be found here: https://docs.google.com/document/d/1DDWy3frMXDTAbk-BfnZ1FdRwuPx6Pl7AWyR4zjqRFZw/edit?tab=t.0#heading=h.giijrtyn1fdx

Reason for change

It's required for RASP.

Implementation details

Test coverage

Other details

github-actions · 2024-11-20T09:40:46Z

Snapshots difference summary

The following differences have been observed in committed snapshots. It is meant to help the reviewer.
The diff is simplistic, so please check some files anyway while we improve it.

8 occurrences of :

-      _dd.appsec.json: {"triggers":[{"rule":{"id":"rasp-932-100","name":"Shell injection exploit","tags":{"category":"vulnerability_trigger","type":"command_injection"}},"rule_matches":[{"operator":"shi_detector","operator_value":"","parameters":[{"address":null,"highlight":[";evilCommand"],"key_path":null,"value":null}]}],"span_id": XXX}]},
+      _dd.appsec.json: {"triggers":[{"rule":{"id":"rasp-932-100","name":"Shell command injection exploit","tags":{"category":"vulnerability_trigger","type":"command_injection"}},"rule_matches":[{"operator":"shi_detector","operator_value":"","parameters":[{"address":null,"highlight":[";evilCommand"],"key_path":null,"value":null}]}],"span_id": XXX}]},

datadog-ddstaging · 2024-11-20T09:58:27Z

Datadog Report

Branch report: nacho/CommandInjection
Commit report: 2951252
Test service: dd-trace-dotnet

✅ 0 Failed, 458002 Passed, 2839 Skipped, 20h 9m 18.79s Total Time

andrewlock · 2024-11-20T10:23:44Z

Execution-Time Benchmarks Report ⏱️

Execution-time results for samples comparing the following branches/commits:

Execution-time benchmarks measure the whole time it takes to execute a program. And are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are shown in red. The following thresholds were used for comparing the execution times:

Welch test with statistical test for significance of 5%
Only results indicating a difference greater than 5% and 5 ms are considered.

Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard.

Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph).

gantt
    title Execution time (ms) FakeDbCommand (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6323) - mean (69ms)  : 66, 72
     .   : milestone, 69,
    master - mean (69ms)  : 66, 73
     .   : milestone, 69,

    section CallTarget+Inlining+NGEN
    This PR (6323) - mean (982ms)  : 960, 1004
     .   : milestone, 982,
    master - mean (978ms)  : 956, 1000
     .   : milestone, 978,

gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6323) - mean (109ms)  : 106, 111
     .   : milestone, 109,
    master - mean (108ms)  : 106, 110
     .   : milestone, 108,

    section CallTarget+Inlining+NGEN
    This PR (6323) - mean (684ms)  : 668, 700
     .   : milestone, 684,
    master - mean (681ms)  : 666, 696
     .   : milestone, 681,

gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6323) - mean (92ms)  : 91, 94
     .   : milestone, 92,
    master - mean (92ms)  : 90, 94
     .   : milestone, 92,

    section CallTarget+Inlining+NGEN
    This PR (6323) - mean (640ms)  : 622, 659
     .   : milestone, 640,
    master - mean (635ms)  : 619, 651
     .   : milestone, 635,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6323) - mean (192ms)  : 186, 198
     .   : milestone, 192,
    master - mean (192ms)  : 187, 197
     .   : milestone, 192,

    section CallTarget+Inlining+NGEN
    This PR (6323) - mean (1,100ms)  : 1074, 1126
     .   : milestone, 1100,
    master - mean (1,100ms)  : 1070, 1131
     .   : milestone, 1100,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6323) - mean (280ms)  : 274, 285
     .   : milestone, 280,
    master - mean (279ms)  : 275, 284
     .   : milestone, 279,

    section CallTarget+Inlining+NGEN
    This PR (6323) - mean (879ms)  : 859, 899
     .   : milestone, 879,
    master - mean (879ms)  : 857, 901
     .   : milestone, 879,

gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6323) - mean (268ms)  : 264, 272
     .   : milestone, 268,
    master - mean (269ms)  : 264, 274
     .   : milestone, 269,

    section CallTarget+Inlining+NGEN
    This PR (6323) - mean (864ms)  : 826, 901
     .   : milestone, 864,
    master - mean (858ms)  : 828, 889
     .   : milestone, 858,

andrewlock · 2024-11-20T11:50:34Z

Benchmarks Report for tracer 🐌

Benchmarks for #6323 compared to master:

1 benchmarks are faster, with geometric mean 1.120
1 benchmarks are slower, with geometric mean 1.131
All benchmarks have the same allocations

The following thresholds were used for comparing the benchmark speeds:

Mann–Whitney U test with statistical test for significance of 5%
Only results indicating a difference greater than 10% and 0.3 ns are considered.

Allocation changes below 0.5% are ignored.

Benchmark details

Benchmarks.Trace.ActivityBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Gen 2	Allocated
master	`StartStopWithChild`	net6.0	8.21μs	44.8ns	269ns	0.0156	0.00779	0	5.61 KB
master	`StartStopWithChild`	netcoreapp3.1	10μs	53.8ns	304ns	0.015	0.00499	0	5.8 KB
master	`StartStopWithChild`	net472	16.3μs	35.8ns	139ns	1.03	0.3	0.0974	6.22 KB
#6323	`StartStopWithChild`	net6.0	7.93μs	43.4ns	271ns	0.0155	0.00388	0	5.61 KB
#6323	`StartStopWithChild`	netcoreapp3.1	10.3μs	57.3ns	358ns	0.0144	0.00481	0	5.8 KB
#6323	`StartStopWithChild`	net472	16.2μs	34ns	132ns	1.04	0.3	0.0971	6.21 KB

Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	494μs	370ns	1.38μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	654μs	285ns	1.1μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	net472	843μs	355ns	1.28μs	0.422	3.3 KB
#6323	`WriteAndFlushEnrichedTraces`	net6.0	520μs	206ns	797ns	0	2.7 KB
#6323	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	647μs	475ns	1.84μs	0	2.7 KB
#6323	`WriteAndFlushEnrichedTraces`	net472	852μs	252ns	942ns	0.425	3.3 KB

Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendRequest`	net6.0	147μs	794ns	4.49μs	0.152	14.47 KB
master	`SendRequest`	netcoreapp3.1	159μs	848ns	5.3μs	0.157	17.27 KB
master	`SendRequest`	net472	0.000771ns	0.000473ns	0.00171ns	0	0 b
#6323	`SendRequest`	net6.0	140μs	741ns	3.63μs	0.154	14.47 KB
#6323	`SendRequest`	netcoreapp3.1	165μs	904ns	5.35μs	0.161	17.27 KB
#6323	`SendRequest`	net472	0ns	0ns	0ns	0	0 b

Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Gen 2	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	566μs	2.67μs	9.99μs	0.556	0	0	41.72 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	666μs	3.33μs	15.6μs	0.322	0	0	41.91 KB
master	`WriteAndFlushEnrichedTraces`	net472	864μs	2.96μs	11.1μs	8.45	2.53	0.422	53.29 KB
#6323	`WriteAndFlushEnrichedTraces`	net6.0	553μs	1.78μs	6.9μs	0.293	0	0	41.53 KB
#6323	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	644μs	3.03μs	11.8μs	0.314	0	0	41.77 KB
#6323	`WriteAndFlushEnrichedTraces`	net472	836μs	1.81μs	6.79μs	8.08	2.55	0.425	53.28 KB

Benchmarks.Trace.DbCommandBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6323

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.DbCommandBenchmark.ExecuteNonQuery‑net6.0	1.120	1,408.04	1,257.52

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`ExecuteNonQuery`	net6.0	1.41μs	1.48ns	5.75ns	0.0141	0	1.02 KB
master	`ExecuteNonQuery`	netcoreapp3.1	1.83μs	1.44ns	5.58ns	0.0136	0	1.02 KB
master	`ExecuteNonQuery`	net472	2.11μs	1.23ns	4.74ns	0.157	0.00106	987 B
#6323	`ExecuteNonQuery`	net6.0	1.26μs	1.25ns	4.7ns	0.0145	0	1.02 KB
#6323	`ExecuteNonQuery`	netcoreapp3.1	1.76μs	1.7ns	6.37ns	0.013	0	1.02 KB
#6323	`ExecuteNonQuery`	net472	2.1μs	2.12ns	8.21ns	0.157	0.00104	987 B

Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`CallElasticsearch`	net6.0	1.3μs	1.29ns	5.01ns	0.0138	976 B
master	`CallElasticsearch`	netcoreapp3.1	1.63μs	1.16ns	4.51ns	0.0132	976 B
master	`CallElasticsearch`	net472	2.46μs	2.19ns	8.2ns	0.158	995 B
master	`CallElasticsearchAsync`	net6.0	1.37μs	0.742ns	2.78ns	0.0129	952 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.64μs	0.853ns	3.3ns	0.0141	1.02 KB
master	`CallElasticsearchAsync`	net472	2.74μs	1.86ns	7.2ns	0.166	1.05 KB
#6323	`CallElasticsearch`	net6.0	1.25μs	2.06ns	7.98ns	0.0137	976 B
#6323	`CallElasticsearch`	netcoreapp3.1	1.58μs	1.14ns	4.4ns	0.0134	976 B
#6323	`CallElasticsearch`	net472	2.53μs	0.998ns	3.46ns	0.158	995 B
#6323	`CallElasticsearchAsync`	net6.0	1.39μs	2.31ns	8.94ns	0.0132	952 B
#6323	`CallElasticsearchAsync`	netcoreapp3.1	1.66μs	0.645ns	2.41ns	0.0141	1.02 KB
#6323	`CallElasticsearchAsync`	net472	2.71μs	2.14ns	7.7ns	0.166	1.05 KB

Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteAsync`	net6.0	1.37μs	1.07ns	4.02ns	0.0138	952 B
master	`ExecuteAsync`	netcoreapp3.1	1.66μs	1.1ns	4.1ns	0.0132	952 B
master	`ExecuteAsync`	net472	1.78μs	0.578ns	2.16ns	0.145	915 B
#6323	`ExecuteAsync`	net6.0	1.39μs	1.14ns	4.26ns	0.0132	952 B
#6323	`ExecuteAsync`	netcoreapp3.1	1.68μs	0.804ns	3.11ns	0.0127	952 B
#6323	`ExecuteAsync`	net472	1.83μs	0.318ns	1.19ns	0.145	915 B

Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendAsync`	net6.0	4.39μs	1.85ns	7.18ns	0.0329	2.31 KB
master	`SendAsync`	netcoreapp3.1	5.42μs	2.55ns	9.53ns	0.0378	2.85 KB
master	`SendAsync`	net472	7.39μs	1.71ns	6.4ns	0.493	3.12 KB
#6323	`SendAsync`	net6.0	4.37μs	2.03ns	7.31ns	0.0306	2.31 KB
#6323	`SendAsync`	netcoreapp3.1	5.44μs	2.19ns	8.2ns	0.0365	2.85 KB
#6323	`SendAsync`	net472	7.29μs	1.87ns	7.23ns	0.496	3.12 KB

Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	1.47μs	0.71ns	2.66ns	0.0235	1.64 KB
master	`EnrichedLog`	netcoreapp3.1	2.36μs	1.4ns	5.24ns	0.0213	1.64 KB
master	`EnrichedLog`	net472	2.57μs	1.07ns	4.15ns	0.249	1.57 KB
#6323	`EnrichedLog`	net6.0	1.46μs	0.706ns	2.73ns	0.0227	1.64 KB
#6323	`EnrichedLog`	netcoreapp3.1	2.16μs	3.12ns	12.1ns	0.0215	1.64 KB
#6323	`EnrichedLog`	net472	2.62μs	1.12ns	4.34ns	0.249	1.57 KB

Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EnrichedLog`	net6.0	120μs	119ns	461ns	0	0	4.28 KB
master	`EnrichedLog`	netcoreapp3.1	126μs	105ns	394ns	0	0	4.28 KB
master	`EnrichedLog`	net472	151μs	179ns	694ns	0.676	0.225	4.46 KB
#6323	`EnrichedLog`	net6.0	121μs	130ns	502ns	0.0606	0	4.28 KB
#6323	`EnrichedLog`	netcoreapp3.1	124μs	124ns	480ns	0	0	4.28 KB
#6323	`EnrichedLog`	net472	151μs	206ns	797ns	0.678	0.226	4.46 KB

Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	3.02μs	1.21ns	4.18ns	0.0318	2.2 KB
master	`EnrichedLog`	netcoreapp3.1	4.15μs	1.82ns	7.04ns	0.0296	2.2 KB
master	`EnrichedLog`	net472	5.04μs	1.36ns	5.29ns	0.319	2.02 KB
#6323	`EnrichedLog`	net6.0	3.11μs	1.68ns	6.52ns	0.031	2.2 KB
#6323	`EnrichedLog`	netcoreapp3.1	4.1μs	1.45ns	5.41ns	0.0288	2.2 KB
#6323	`EnrichedLog`	net472	4.92μs	10.6ns	40.9ns	0.318	2.02 KB

Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendReceive`	net6.0	1.34μs	0.789ns	2.85ns	0.0154	1.14 KB
master	`SendReceive`	netcoreapp3.1	1.77μs	0.496ns	1.92ns	0.0151	1.14 KB
master	`SendReceive`	net472	2.08μs	0.886ns	3.32ns	0.184	1.16 KB
#6323	`SendReceive`	net6.0	1.45μs	0.516ns	1.93ns	0.016	1.14 KB
#6323	`SendReceive`	netcoreapp3.1	1.83μs	1.1ns	4.25ns	0.0155	1.14 KB
#6323	`SendReceive`	net472	2.04μs	0.669ns	2.5ns	0.183	1.16 KB

Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	2.81μs	1.23ns	4.77ns	0.0224	1.6 KB
master	`EnrichedLog`	netcoreapp3.1	3.78μs	1.56ns	6.05ns	0.0209	1.65 KB
master	`EnrichedLog`	net472	4.43μs	1.93ns	7.21ns	0.322	2.04 KB
#6323	`EnrichedLog`	net6.0	2.8μs	0.943ns	3.65ns	0.0222	1.6 KB
#6323	`EnrichedLog`	netcoreapp3.1	3.97μs	1.29ns	4.83ns	0.0219	1.65 KB
#6323	`EnrichedLog`	net472	4.44μs	2.56ns	9.57ns	0.323	2.04 KB

Benchmarks.Trace.SpanBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #6323

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑net472	1.131	643.52	727.61

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net6.0	394ns	0.255ns	0.987ns	0.00812	576 B
master	`StartFinishSpan`	netcoreapp3.1	564ns	0.426ns	1.65ns	0.00761	576 B
master	`StartFinishSpan`	net472	643ns	0.407ns	1.58ns	0.0917	578 B
master	`StartFinishScope`	net6.0	496ns	0.208ns	0.805ns	0.00968	696 B
master	`StartFinishScope`	netcoreapp3.1	784ns	0.347ns	1.25ns	0.00942	696 B
master	`StartFinishScope`	net472	868ns	0.668ns	2.59ns	0.104	658 B
#6323	`StartFinishSpan`	net6.0	396ns	0.15ns	0.58ns	0.00813	576 B
#6323	`StartFinishSpan`	netcoreapp3.1	623ns	0.289ns	1.12ns	0.00776	576 B
#6323	`StartFinishSpan`	net472	728ns	1.25ns	4.85ns	0.0917	578 B
#6323	`StartFinishScope`	net6.0	489ns	0.275ns	1.06ns	0.00978	696 B
#6323	`StartFinishScope`	netcoreapp3.1	764ns	0.632ns	2.45ns	0.0096	696 B
#6323	`StartFinishScope`	net472	796ns	0.532ns	2.06ns	0.104	658 B

Benchmarks.Trace.TraceAnnotationsBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunOnMethodBegin`	net6.0	653ns	0.518ns	2ns	0.00989	696 B
master	`RunOnMethodBegin`	netcoreapp3.1	894ns	0.626ns	2.42ns	0.0094	696 B
master	`RunOnMethodBegin`	net472	1.17μs	0.922ns	3.57ns	0.104	658 B
#6323	`RunOnMethodBegin`	net6.0	661ns	0.633ns	2.45ns	0.00965	696 B
#6323	`RunOnMethodBegin`	netcoreapp3.1	937ns	4.37ns	16.9ns	0.00952	696 B
#6323	`RunOnMethodBegin`	net472	1.17μs	0.479ns	1.73ns	0.104	658 B

NachoEchevarria · 2024-11-22T12:16:23Z

tracer/test/Datadog.Trace.Tests/Telemetry/Metrics/common_metrics.json

@@ -728,7 +728,8 @@
    "rasp.rule.eval": {
      "tags": [
        "waf_version",
-        "rule_type"


The values has been updated in the dd-go repo common_metrics file as well

I'm assumingthis is something that was agreed across all langs? 🙂 If not, we should make sure to flag/discuss it first!

Yes, it's a new flag that was introduced to distinguish between command injection and shell injection vulnerabilities and it's defined for all the libraries in the RFC.

andrewlock · 2024-11-22T13:15:05Z

Benchmarks Report for appsec 🐌

Benchmarks for #6323 compared to master:

1 benchmarks are slower, with geometric mean 1.149
1 benchmarks have fewer allocations
1 benchmarks have more allocations

The following thresholds were used for comparing the benchmark speeds:

Mann–Whitney U test with statistical test for significance of 5%
Only results indicating a difference greater than 10% and 0.3 ns are considered.

Allocation changes below 0.5% are ignored.

Benchmark details

Benchmarks.Trace.Asm.AppSecBodyBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`AllCycleSimpleBody`	net6.0	211μs	162ns	608ns	2.87	0	204.98 KB
master	`AllCycleSimpleBody`	netcoreapp3.1	314μs	108ns	373ns	2.81	0	212.23 KB
master	`AllCycleSimpleBody`	net472	277μs	212ns	820ns	38	2.87	239.39 KB
master	`AllCycleMoreComplexBody`	net6.0	220μs	71.5ns	267ns	2.97	0	208.49 KB
master	`AllCycleMoreComplexBody`	netcoreapp3.1	317μs	134ns	519ns	2.84	0	215.65 KB
master	`AllCycleMoreComplexBody`	net472	281μs	421ns	1.63μs	38.6	2.82	242.91 KB
master	`ObjectExtractorSimpleBody`	net6.0	138ns	0.159ns	0.617ns	0.00396	0	280 B
master	`ObjectExtractorSimpleBody`	netcoreapp3.1	199ns	0.139ns	0.519ns	0.00374	0	272 B
master	`ObjectExtractorSimpleBody`	net472	212ns	0.103ns	0.385ns	0.0445	0	281 B
master	`ObjectExtractorMoreComplexBody`	net6.0	2.94μs	1.41ns	5.09ns	0.0526	0	3.78 KB
master	`ObjectExtractorMoreComplexBody`	netcoreapp3.1	3.73μs	3.13ns	11.7ns	0.05	0	3.69 KB
master	`ObjectExtractorMoreComplexBody`	net472	4.24μs	4.48ns	17.3ns	0.601	0.00633	3.8 KB
#6323	`AllCycleSimpleBody`	net6.0	212μs	93.5ns	337ns	2.87	0	204.98 KB
#6323	`AllCycleSimpleBody`	netcoreapp3.1	321μs	286ns	1.03μs	2.88	0	212.23 KB
#6323	`AllCycleSimpleBody`	net472	279μs	186ns	698ns	38	2.8	239.39 KB
#6323	`AllCycleMoreComplexBody`	net6.0	219μs	71.2ns	247ns	2.95	0	208.49 KB
#6323	`AllCycleMoreComplexBody`	netcoreapp3.1	335μs	109ns	392ns	2.84	0	215.65 KB
#6323	`AllCycleMoreComplexBody`	net472	283μs	207ns	803ns	38.5	2.84	242.91 KB
#6323	`ObjectExtractorSimpleBody`	net6.0	139ns	0.171ns	0.64ns	0.00392	0	280 B
#6323	`ObjectExtractorSimpleBody`	netcoreapp3.1	197ns	0.106ns	0.368ns	0.00377	0	272 B
#6323	`ObjectExtractorSimpleBody`	net472	211ns	0.157ns	0.609ns	0.0446	0	281 B
#6323	`ObjectExtractorMoreComplexBody`	net6.0	2.93μs	2.28ns	8.21ns	0.0534	0	3.78 KB
#6323	`ObjectExtractorMoreComplexBody`	netcoreapp3.1	3.8μs	1.93ns	7.2ns	0.0502	0	3.69 KB
#6323	`ObjectExtractorMoreComplexBody`	net472	4.29μs	3.32ns	12.9ns	0.602	0.00641	3.8 KB

Benchmarks.Trace.Asm.AppSecEncoderBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #6323

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.Asm.AppSecEncoderBenchmark.EncodeLegacyArgs‑net6.0	1.149	72,259.88	83,042.79

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EncodeArgs`	net6.0	37.1μs	15.9ns	59.7ns	0.447	0	32.4 KB
master	`EncodeArgs`	netcoreapp3.1	54.1μs	20.7ns	74.5ns	0.432	0	32.4 KB
master	`EncodeArgs`	net472	68.6μs	104ns	402ns	5.16	0.0684	32.5 KB
master	`EncodeLegacyArgs`	net6.0	72.4μs	55.7ns	243ns	0	0	2.14 KB
master	`EncodeLegacyArgs`	netcoreapp3.1	103μs	123ns	475ns	0	0	2.14 KB
master	`EncodeLegacyArgs`	net472	157μs	53.5ns	207ns	0.314	0	2.15 KB
#6323	`EncodeArgs`	net6.0	37.8μs	14.6ns	54.6ns	0.453	0	32.4 KB
#6323	`EncodeArgs`	netcoreapp3.1	54.7μs	41.4ns	155ns	0.436	0	32.4 KB
#6323	`EncodeArgs`	net472	66.4μs	30.1ns	117ns	5.16	0.0666	32.5 KB
#6323	`EncodeLegacyArgs`	net6.0	82.3μs	384ns	1.49μs	0	0	2.14 KB
#6323	`EncodeLegacyArgs`	netcoreapp3.1	105μs	126ns	489ns	0	0	2.14 KB
#6323	`EncodeLegacyArgs`	net472	155μs	94.9ns	368ns	0.31	0	2.15 KB

Benchmarks.Trace.Asm.AppSecWafBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunWafRealisticBenchmark`	net6.0	184μs	129ns	498ns	0	2.44 KB
master	`RunWafRealisticBenchmark`	netcoreapp3.1	199μs	201ns	779ns	0	2.39 KB
master	`RunWafRealisticBenchmark`	net472	210μs	107ns	401ns	0.316	2.46 KB
master	`RunWafRealisticBenchmarkWithAttack`	net6.0	123μs	149ns	577ns	0	1.47 KB
master	`RunWafRealisticBenchmarkWithAttack`	netcoreapp3.1	131μs	134ns	501ns	0	1.46 KB
master	`RunWafRealisticBenchmarkWithAttack`	net472	141μs	118ns	442ns	0.215	1.49 KB
#6323	`RunWafRealisticBenchmark`	net6.0	184μs	685ns	3.69μs	0	2.44 KB
#6323	`RunWafRealisticBenchmark`	netcoreapp3.1	193μs	132ns	512ns	0	2.39 KB
#6323	`RunWafRealisticBenchmark`	net472	208μs	99.1ns	384ns	0.312	2.46 KB
#6323	`RunWafRealisticBenchmarkWithAttack`	net6.0	122μs	125ns	483ns	0	1.47 KB
#6323	`RunWafRealisticBenchmarkWithAttack`	netcoreapp3.1	130μs	87.4ns	327ns	0	1.46 KB
#6323	`RunWafRealisticBenchmarkWithAttack`	net472	141μs	55.3ns	207ns	0.211	1.49 KB

Benchmarks.Trace.Iast.StringAspectsBenchmark - Same speed ✔️ More allocations ⚠️

More allocations ⚠️ in #6323

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1	252.65 KB	254.13 KB	1.48 KB	0.59%

Fewer allocations 🎉 in #6323

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net6.0	264.92 KB	259.13 KB	-5.79 KB	-2.19%

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Allocated
master	`StringConcatBenchmark`	net6.0	59.1μs	714ns	7.07μs	43.44 KB
master	`StringConcatBenchmark`	netcoreapp3.1	54.3μs	252ns	975ns	42.64 KB
master	`StringConcatBenchmark`	net472	38.3μs	115ns	415ns	59.07 KB
master	`StringConcatAspectBenchmark`	net6.0	295μs	4.97μs	48.4μs	264.92 KB
master	`StringConcatAspectBenchmark`	netcoreapp3.1	336μs	1.85μs	13.2μs	252.65 KB
master	`StringConcatAspectBenchmark`	net472	280μs	4.72μs	44.7μs	278.53 KB
#6323	`StringConcatBenchmark`	net6.0	62.5μs	739ns	7.32μs	43.44 KB
#6323	`StringConcatBenchmark`	netcoreapp3.1	58.7μs	704ns	6.94μs	42.64 KB
#6323	`StringConcatBenchmark`	net472	38.2μs	86.2ns	311ns	59.07 KB
#6323	`StringConcatAspectBenchmark`	net6.0	311μs	5.86μs	58.3μs	259.13 KB
#6323	`StringConcatAspectBenchmark`	netcoreapp3.1	341μs	1.9μs	11.5μs	254.13 KB
#6323	`StringConcatAspectBenchmark`	net472	288μs	6.73μs	66.3μs	278.53 KB

anna-git

a few NITs but LGTM 🥳

tracer/src/Datadog.Trace/AppSec/Rasp/RaspShellInjectionHelper.cs

anna-git · 2024-11-28T13:24:46Z

tracer/src/Datadog.Trace/AppSec/WafEncoding/Encoder.cs

-            while (enumerator.MoveNext())
-            {
-                if (enumerator.Current != null)
+                while (enumerator.MoveNext())


NIT maybe instead which would allow removing some duplicated codes, could be:

Suggested change

while (enumerator.MoveNext())

var canMoveNext = enumerator.MoveNext();

while (canMoveNext)

{

if (enumerator.Current != null)

{

FormatArgsInternal(enumerator.Current, sb);

canMoveNext = enumerator.MoveNext();

if(canMoveNext)

{

sb.Append(", ");

}

}

}

also there's also a LegacyEncoder that's being used by default

I have updated both. Thanks!

tracer/src/Datadog.Trace/RemoteConfigurationManagement/RcmCapabilitiesIndices.cs

Co-authored-by: Anna <[email protected]>

…bilitiesIndices.cs Co-authored-by: Anna <[email protected]>

andrewlock · 2024-11-28T18:58:23Z

Throughput/Crank Report ⚡

Throughput results for AspNetCoreSimpleController comparing the following branches/commits:

Cases where throughput results for the PR are worse than latest master (5% drop or greater), results are shown in red.

Note that these results are based on a single point-in-time result for each branch. For full results, see one of the many, many dashboards!

gantt
    title Throughput Linux x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6323) (11.205M)   : 0, 11205041
    master (11.088M)   : 0, 11088184
    benchmarks/2.9.0 (11.033M)   : 0, 11032866

    section Automatic
    This PR (6323) (7.242M)   : 0, 7241694
    master (7.254M)   : 0, 7254128
    benchmarks/2.9.0 (7.786M)   : 0, 7785853

    section Trace stats
    master (7.516M)   : 0, 7515745

    section Manual
    master (11.014M)   : 0, 11013989

    section Manual + Automatic
    This PR (6323) (6.759M)   : 0, 6758799
    master (6.700M)   : 0, 6700166

    section DD_TRACE_ENABLED=0
    master (10.228M)   : 0, 10228208

gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6323) (9.622M)   : 0, 9622085
    master (9.438M)   : 0, 9438272
    benchmarks/2.9.0 (9.495M)   : 0, 9494821

    section Automatic
    This PR (6323) (6.312M)   : 0, 6312223
    master (6.384M)   : 0, 6384110

    section Trace stats
    master (6.758M)   : 0, 6758045

    section Manual
    master (9.394M)   : 0, 9393606

    section Manual + Automatic
    This PR (6323) (5.984M)   : 0, 5984044
    master (6.041M)   : 0, 6040522

    section DD_TRACE_ENABLED=0
    master (8.691M)   : 0, 8690632

gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6323) (9.262M)   : 0, 9261767
    master (9.438M)   : 0, 9438032
    benchmarks/2.9.0 (10.020M)   : 0, 10019592

    section Automatic
    This PR (6323) (5.968M)   : 0, 5968145
    master (6.073M)   : 0, 6073208
    benchmarks/2.9.0 (7.255M)   : 0, 7255257

    section Trace stats
    master (6.747M)   : 0, 6747095

    section Manual
    master (9.357M)   : 0, 9356938

    section Manual + Automatic
    This PR (6323) (5.509M)   : 0, 5509372
    master (5.605M)   : 0, 5604930

    section DD_TRACE_ENABLED=0
    master (8.787M)   : 0, 8787187

andrewlock

LGTM, just a few questions and nit comments 🙂

tracer/src/Datadog.Trace/AppSec/Rasp/RaspShellInjectionHelper.cs

tracer/src/Datadog.Trace/AppSec/Waf/RCMCapabilitiesHelper.cs

andrewlock · 2024-12-09T13:40:38Z

tracer/src/Datadog.Trace/AppSec/WafEncoding/Encoder.cs

+            using var enumerator = objs.GetEnumerator();
+            var canMoveNext = enumerator.MoveNext();
+            while (canMoveNext && enumerator.Current != null)


nit: I feel like there should be some unit tests for this Encoder - it would be easy to do, and is probably worth testing 🙂 It's kind of hard to visualize this tbh. For example, this stops itterating as soon as the objs contains a null, while the previous version doesn't. And I'm not sure what the expected behaviour is 🤔

This method creates some debug messages when debug is enabled. I have added some unit tests. Basically, this change increases readability by adding commas between elements and improves performance.

tracer/src/Datadog.Trace/AppSec/WafEncoding/EncoderLegacy.cs

andrewlock · 2024-12-09T13:43:35Z

tracer/test/Datadog.Trace.Security.Unit.Tests/RASP/RaspWafTests.cs

+    private void EnableDebugInfo(bool enable)
+    {
+        if (enable)
+        {
+            DatadogLogging.SetLogLevel(LogEventLevel.Debug);
+        }
+    }


I don't think you should do this. This sets global values. You really shouldn't be doing that in unit tests, as it just tends to cause problems in the long run. If this was temporary for debugging purposes, that's fine, but we should remove it I think 🙂

andrewlock · 2024-12-09T13:44:35Z

tracer/test/Datadog.Trace.Security.Unit.Tests/Utils/WafLibraryRequiredTest.cs

@@ -51,20 +51,21 @@ internal static ConfigurationState UpdateConfigurationState(string? ruleFile = n
        return CreateConfigurationState(creation: false, ruleFile, ruleOverrides, rulesData, ruleSet, actions);
    }

-    internal InitResult CreateWaf(bool useUnsafeEncoder = false, string? ruleFile = null, string? obfuscationParameterKeyRegex = null, string? obfuscationParameterValueRegex = null, bool expectWafNull = false)
+    internal InitResult CreateWaf(bool useUnsafeEncoder = false, string? ruleFile = null, string? obfuscationParameterKeyRegex = null, string? obfuscationParameterValueRegex = null, bool expectWafNull = false, bool wafDebugEnabled = false)


Do you actually set this to true anywhere 🤔 It doesn't look like it?

No, this debug mode was added as an easy way to debug locally when needed but not to be run in CI. I can undo these changes though.

was added as an easy way to debug locally when needed but not to be run in CI.

Ah ok, that makes sense 👍

I can undo these changes though.

No strong feelings, just wanted to check the change was intentional 🙂

andrewlock · 2024-12-09T13:45:41Z

tracer/test/Datadog.Trace.Tests/Telemetry/Metrics/common_metrics.json

@@ -728,7 +728,8 @@
    "rasp.rule.eval": {
      "tags": [
        "waf_version",
-        "rule_type"


I'm assumingthis is something that was agreed across all langs? 🙂 If not, we should make sure to flag/discuss it first!

andrewlock · 2024-12-09T13:50:32Z

Additional question, the doc says

tracers supporting the new command injection heuristic must be released with the v1.13.3 ruleset

Do we have that?

Co-authored-by: Andrew Lock <[email protected]>

andrewlock · 2024-12-10T14:31:29Z

tracer/test/Datadog.Trace.Security.Unit.Tests/EncoderUnitTests.cs

@@ -179,6 +179,15 @@ public void TestMapListDepth(int length, int expectedLength)
        Assert.Equal(expectedLength, CountNestedMapDepth(result));
    }

+    [SkippableTheory]
+    [InlineData(new object[] { "a", 3, new object[] { "b", 6 }, 44 }, "[ a, 3, [ b, 6 ], 44 ]")]
+    [InlineData(new object[] { "aba", "3", new object[] { new object[] { "w" }, "e" },  }, "[ aba, 3, [ [ w ], e ] ]")]


I suggest you add some examples that include null, because I think the behaviour has changed in the refactoring...

e.g. is this actually what you expect?

Suggested change

[InlineData(new object[] { "aba", "3", new object[] { new object[] { "w" }, "e" }, }, "[ aba, 3, [ [ w ], e ] ]")]

[InlineData(new object[] { "aba", "3", new object[] { new object[] { "w" }, "e" }, }, "[ aba, 3, [ [ w ], e ] ]")]

[InlineData(new object[] { "aba", null, "3", new object[] { new object[] { "w" }, "e" }, }, "[ aba ]")]

Yes, you are right! I think that it would be useful also to print the null values as "null" when debuging. I have modified the methods and tests. Thank you!

NachoEchevarria · 2024-12-11T11:01:40Z

Additional question, the doc says

tracers supporting the new command injection heuristic must be released with the v1.13.3 ruleset

Do we have that?

Yes, we updated the ruleset before this PR:
#6287

NachoEchevarria · 2024-12-11T11:01:55Z

Thanks for your feedback and reviews!

## Summary of changes This PR contains the implementation of the RASP command injection vulnerability. Previously, the shell injection vulnerability was implemented, which is similar but only occurs the flag UseShellExecute is set to false in method process.start(). The definition of this new vulnerability can be found here: https://docs.google.com/document/d/1DDWy3frMXDTAbk-BfnZ1FdRwuPx6Pl7AWyR4zjqRFZw/edit?tab=t.0#heading=h.giijrtyn1fdx ## Reason for change It's required for RASP. ## Implementation details ## Test coverage ## Other details   --------- Co-authored-by: Anna <[email protected]> Co-authored-by: Andrew Lock <[email protected]>

NachoEchevarria added 10 commits November 20, 2024 10:38

Update WAF version

760d0e0

Remove not needed

680369e

Update ruleset

abbed4b

Fix rules number in smoke tests. Remove not needed test.

4a1964e

Fix unit test

b4e7dc3

Update rule number in snapshots

b9c37ea

Add RASP command injection unit tests, capability and rulesets

102ba25

Command injection vulnerability implementation

77c9907

Update telemetry

c5c2b53

Integration tests

52bba01

NachoEchevarria force-pushed the nacho/CommandInjection branch from 775b4f6 to 52bba01 Compare November 20, 2024 09:39

NachoEchevarria and others added 4 commits November 20, 2024 14:42

Metric tests

c20e404

Merge branch 'master' into nacho/CommandInjection

10cf4a5

remove windows only unit test

6d6e21c

Add capability support

d0ae8eb

NachoEchevarria commented Nov 22, 2024

View reviewed changes

Undo change in tests

82711d6

NachoEchevarria changed the title ~~Nacho/command injection~~ [ASM] RASP: Command injection vulnerability implementation Nov 22, 2024

NachoEchevarria added the area:asm label Nov 22, 2024

NachoEchevarria marked this pull request as ready for review November 22, 2024 13:52

NachoEchevarria requested review from a team as code owners November 22, 2024 13:52

anna-git approved these changes Nov 28, 2024

View reviewed changes

Update tracer/src/Datadog.Trace/AppSec/Rasp/RaspShellInjectionHelper.cs

3b5b9e2

Co-authored-by: Anna <[email protected]>

NachoEchevarria and others added 2 commits November 28, 2024 14:48

Update tracer/src/Datadog.Trace/RemoteConfigurationManagement/RcmCapa…

6854d5e

…bilitiesIndices.cs Co-authored-by: Anna <[email protected]>

Small encoder improvement

ae3b173

andrewlock approved these changes Dec 9, 2024

View reviewed changes

NachoEchevarria and others added 3 commits December 9, 2024 15:17

Update tracer/src/Datadog.Trace/AppSec/Rasp/RaspShellInjectionHelper.cs

cf799d8

Co-authored-by: Andrew Lock <[email protected]>

Add encoder unit tests

7153cf9

Merge branch 'master' into nacho/CommandInjection

eb30176

andrewlock reviewed Dec 10, 2024

View reviewed changes

update snapshots and encoder unit test

2951252

NachoEchevarria merged commit e645d06 into master Dec 11, 2024
63 of 66 checks passed

NachoEchevarria deleted the nacho/CommandInjection branch December 11, 2024 11:02

github-actions bot added this to the vNext-v3 milestone Dec 11, 2024

andrewlock added type:new-feature area:asm-iast labels Dec 18, 2024

	[InlineData(new object[] { "aba", "3", new object[] { new object[] { "w" }, "e" }, }, "[ aba, 3, [ [ w ], e ] ]")]
	[InlineData(new object[] { "aba", "3", new object[] { new object[] { "w" }, "e" }, }, "[ aba, 3, [ [ w ], e ] ]")]
	[InlineData(new object[] { "aba", null, "3", new object[] { new object[] { "w" }, "e" }, }, "[ aba ]")]

[ASM] RASP: Command injection vulnerability implementation #6323

[ASM] RASP: Command injection vulnerability implementation #6323

Conversation

NachoEchevarria commented Nov 20, 2024 • edited Loading

Summary of changes

Reason for change

Implementation details

Test coverage

Other details

github-actions bot commented Nov 20, 2024 • edited Loading

Snapshots difference summary

datadog-ddstaging bot commented Nov 20, 2024 • edited Loading

Datadog Report

andrewlock commented Nov 20, 2024 • edited Loading

Execution-Time Benchmarks Report ⏱️

andrewlock commented Nov 20, 2024 • edited Loading

Benchmarks Report for tracer 🐌

Benchmark details

Raw results

Raw results

Raw results

Raw results

Faster 🎉 in #6323

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Slower ⚠️ in #6323

Raw results

Raw results

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewlock commented Nov 22, 2024 • edited Loading

Benchmarks Report for appsec 🐌

Benchmark details

Raw results

Slower ⚠️ in #6323

Raw results

Raw results

More allocations ⚠️ in #6323

Fewer allocations 🎉 in #6323

Raw results

anna-git left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewlock commented Nov 28, 2024 • edited Loading

Throughput/Crank Report ⚡

andrewlock left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewlock commented Dec 9, 2024

andrewlock Dec 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NachoEchevarria commented Dec 11, 2024

NachoEchevarria commented Dec 11, 2024

NachoEchevarria commented Nov 20, 2024 •

edited

Loading

github-actions bot commented Nov 20, 2024 •

edited

Loading

datadog-ddstaging bot commented Nov 20, 2024 •

edited

Loading

andrewlock commented Nov 20, 2024 •

edited

Loading

andrewlock commented Nov 20, 2024 •

edited

Loading

andrewlock commented Nov 22, 2024 •

edited

Loading

andrewlock commented Nov 28, 2024 •

edited

Loading

andrewlock Dec 10, 2024 •

edited

Loading