[ASM] Email injection: MimeKit and SimpleEmail #6614

NachoEchevarria · 2025-01-31T15:13:03Z

Summary of changes

This PR adds support for some email libraries that were not instrumented and were not detecting email html injection vulnerabilities.

This PR is the implementation of this RFC

Reason for change

We are currently detecting email injections when using Microsoft's email libraries. These libraries are not the most used though, so we have decided to add support for other libraries as well (MimeKit and Amazon SimpleEmail).

Implementation details

Test coverage

Other details

andrewlock · 2025-01-31T16:47:02Z

Benchmarks Report for appsec 🐌

Benchmarks for #6614 compared to master:

1 benchmarks are faster, with geometric mean 1.294
2 benchmarks are slower, with geometric mean 1.149
2 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 - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #6614

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody‑netcoreapp3.1	1.165	201.93	235.23
Benchmarks.Trace.Asm.AppSecBodyBenchmark.AllCycleMoreComplexBody‑net6.0	1.133	192,211.30	217,705.45

Faster 🎉 in #6614

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody‑net6.0	1.294	178.58	138.03

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`AllCycleSimpleBody`	net6.0	191μs	84.3ns	316ns	2.76	0	195.42 KB
master	`AllCycleSimpleBody`	netcoreapp3.1	296μs	71.8ns	249ns	2.67	0	202.94 KB
master	`AllCycleSimpleBody`	net472	262μs	216ns	808ns	37.1	2.09	233.74 KB
master	`AllCycleMoreComplexBody`	net6.0	192μs	105ns	380ns	2.79	0	198.93 KB
master	`AllCycleMoreComplexBody`	netcoreapp3.1	303μs	161ns	603ns	2.87	0	206.36 KB
master	`AllCycleMoreComplexBody`	net472	264μs	130ns	487ns	37.6	2.11	237.26 KB
master	`ObjectExtractorSimpleBody`	net6.0	179ns	0.111ns	0.402ns	0.00391	0	280 B
master	`ObjectExtractorSimpleBody`	netcoreapp3.1	202ns	0.173ns	0.647ns	0.00369	0	272 B
master	`ObjectExtractorSimpleBody`	net472	208ns	0.109ns	0.395ns	0.0446	0	281 B
master	`ObjectExtractorMoreComplexBody`	net6.0	2.87μs	1.49ns	5.77ns	0.0531	0	3.78 KB
master	`ObjectExtractorMoreComplexBody`	netcoreapp3.1	3.73μs	2.76ns	10.3ns	0.05	0	3.69 KB
master	`ObjectExtractorMoreComplexBody`	net472	4.42μs	3.37ns	12.6ns	0.602	0.00662	3.8 KB
#6614	`AllCycleSimpleBody`	net6.0	210μs	117ns	436ns	2.72	0	195.28 KB
#6614	`AllCycleSimpleBody`	netcoreapp3.1	315μs	113ns	438ns	2.67	0	202.8 KB
#6614	`AllCycleSimpleBody`	net472	278μs	144ns	518ns	37	2.08	233.59 KB
#6614	`AllCycleMoreComplexBody`	net6.0	218μs	106ns	381ns	2.82	0	198.78 KB
#6614	`AllCycleMoreComplexBody`	netcoreapp3.1	327μs	133ns	496ns	2.77	0	206.22 KB
#6614	`AllCycleMoreComplexBody`	net472	285μs	111ns	428ns	37.7	2.14	237.1 KB
#6614	`ObjectExtractorSimpleBody`	net6.0	138ns	0.188ns	0.727ns	0.00397	0	280 B
#6614	`ObjectExtractorSimpleBody`	netcoreapp3.1	235ns	0.215ns	0.805ns	0.00376	0	272 B
#6614	`ObjectExtractorSimpleBody`	net472	208ns	0.089ns	0.321ns	0.0445	0	281 B
#6614	`ObjectExtractorMoreComplexBody`	net6.0	3μs	1.42ns	5.33ns	0.0538	0	3.78 KB
#6614	`ObjectExtractorMoreComplexBody`	netcoreapp3.1	3.7μs	2.15ns	8.03ns	0.0499	0	3.69 KB
#6614	`ObjectExtractorMoreComplexBody`	net472	4.44μs	2.44ns	9.11ns	0.601	0.00666	3.8 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EncodeArgs`	net6.0	37.9μs	23.6ns	81.7ns	0.451	0	32.4 KB
master	`EncodeArgs`	netcoreapp3.1	53.9μs	18.8ns	73ns	0.433	0	32.4 KB
master	`EncodeArgs`	net472	66.1μs	46.2ns	179ns	5.15	0.0661	32.5 KB
master	`EncodeLegacyArgs`	net6.0	79.9μs	30.6ns	110ns	0	0	2.14 KB
master	`EncodeLegacyArgs`	netcoreapp3.1	107μs	276ns	1.07μs	0	0	2.14 KB
master	`EncodeLegacyArgs`	net472	153μs	95.8ns	371ns	0.308	0	2.15 KB
#6614	`EncodeArgs`	net6.0	37μs	26.3ns	102ns	0.463	0	32.4 KB
#6614	`EncodeArgs`	netcoreapp3.1	54.3μs	22.2ns	85.8ns	0.431	0	32.4 KB
#6614	`EncodeArgs`	net472	65.9μs	66.4ns	248ns	5.15	0.0657	32.5 KB
#6614	`EncodeLegacyArgs`	net6.0	78.8μs	450ns	3.28μs	0	0	2.14 KB
#6614	`EncodeLegacyArgs`	netcoreapp3.1	105μs	92.7ns	359ns	0	0	2.14 KB
#6614	`EncodeLegacyArgs`	net472	152μs	104ns	403ns	0.307	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	175μs	93.2ns	361ns	0	2.54 KB
master	`RunWafRealisticBenchmark`	netcoreapp3.1	186μs	190ns	734ns	0	2.49 KB
master	`RunWafRealisticBenchmark`	net472	201μs	76.9ns	288ns	0.403	2.55 KB
master	`RunWafRealisticBenchmarkWithAttack`	net6.0	116μs	53.5ns	200ns	0	1.57 KB
master	`RunWafRealisticBenchmarkWithAttack`	netcoreapp3.1	122μs	41.2ns	148ns	0	1.55 KB
master	`RunWafRealisticBenchmarkWithAttack`	net472	134μs	31.3ns	113ns	0.2	1.58 KB
#6614	`RunWafRealisticBenchmark`	net6.0	176μs	216ns	838ns	0	2.54 KB
#6614	`RunWafRealisticBenchmark`	netcoreapp3.1	192μs	403ns	1.56μs	0	2.49 KB
#6614	`RunWafRealisticBenchmark`	net472	202μs	37ns	138ns	0.399	2.55 KB
#6614	`RunWafRealisticBenchmarkWithAttack`	net6.0	117μs	205ns	795ns	0	1.57 KB
#6614	`RunWafRealisticBenchmarkWithAttack`	netcoreapp3.1	124μs	104ns	402ns	0	1.55 KB
#6614	`RunWafRealisticBenchmarkWithAttack`	net472	133μs	36.9ns	138ns	0.2	1.58 KB

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

More allocations ⚠️ in #6614

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net472	57.3 KB	59.55 KB	2.26 KB	3.94%
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1	254.23 KB	262.69 KB	8.46 KB	3.33%

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Allocated
master	`StringConcatBenchmark`	net6.0	59.9μs	687ns	6.87μs	43.44 KB
master	`StringConcatBenchmark`	netcoreapp3.1	60.9μs	787ns	7.83μs	42.64 KB
master	`StringConcatBenchmark`	net472	37.9μs	156ns	812ns	57.3 KB
master	`StringConcatAspectBenchmark`	net6.0	318μs	1.34μs	8.48μs	253.82 KB
master	`StringConcatAspectBenchmark`	netcoreapp3.1	349μs	1.97μs	13.3μs	254.23 KB
master	`StringConcatAspectBenchmark`	net472	293μs	6.62μs	63.8μs	278.53 KB
#6614	`StringConcatBenchmark`	net6.0	52μs	211ns	789ns	43.44 KB
#6614	`StringConcatBenchmark`	netcoreapp3.1	54.2μs	237ns	1.06μs	42.64 KB
#6614	`StringConcatBenchmark`	net472	37.2μs	106ns	396ns	59.55 KB
#6614	`StringConcatAspectBenchmark`	net6.0	292μs	3.96μs	36.9μs	253.47 KB
#6614	`StringConcatAspectBenchmark`	netcoreapp3.1	358μs	1.9μs	9.67μs	262.69 KB
#6614	`StringConcatAspectBenchmark`	net472	299μs	8.19μs	80.2μs	278.53 KB

andrewlock · 2025-01-31T18:20:18Z

Benchmarks Report for tracer 🐌

Benchmarks for #6614 compared to master:

2 benchmarks are faster, with geometric mean 1.127
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.19μs	46.7ns	353ns	0.0155	0.00387	0	5.61 KB
master	`StartStopWithChild`	netcoreapp3.1	10.4μs	56ns	317ns	0.0201	0.00503	0	5.81 KB
master	`StartStopWithChild`	net472	16.3μs	58.4ns	211ns	1.05	0.33	0.0966	6.21 KB
#6614	`StartStopWithChild`	net6.0	8.12μs	46ns	328ns	0.0156	0.00779	0	5.61 KB
#6614	`StartStopWithChild`	netcoreapp3.1	10.4μs	59.9ns	479ns	0.0214	0.00534	0	5.8 KB
#6614	`StartStopWithChild`	net472	16μs	64.6ns	250ns	1.04	0.304	0.101	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	488μs	459ns	1.78μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	647μs	611ns	2.37μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	net472	859μs	807ns	3.12μs	0.422	3.3 KB
#6614	`WriteAndFlushEnrichedTraces`	net6.0	499μs	308ns	1.11μs	0	2.7 KB
#6614	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	685μs	612ns	2.37μs	0	2.7 KB
#6614	`WriteAndFlushEnrichedTraces`	net472	851μs	465ns	1.74μs	0.428	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	130μs	424ns	1.64μs	0.198	14.47 KB
master	`SendRequest`	netcoreapp3.1	143μs	485ns	1.88μs	0.208	17.27 KB
master	`SendRequest`	net472	0.000683ns	0.00027ns	0.00104ns	0	0 b
#6614	`SendRequest`	net6.0	133μs	478ns	1.85μs	0.197	14.47 KB
#6614	`SendRequest`	netcoreapp3.1	153μs	162ns	627ns	0.154	17.27 KB
#6614	`SendRequest`	net472	0.000347ns	0.000168ns	0.000606ns	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	582μs	2.8μs	10.9μs	0.553	0	0	41.61 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	699μs	3.98μs	32.3μs	0.338	0	0	41.72 KB
master	`WriteAndFlushEnrichedTraces`	net472	864μs	4.06μs	17.2μs	8.45	2.53	0.422	53.33 KB
#6614	`WriteAndFlushEnrichedTraces`	net6.0	616μs	3.51μs	31.6μs	0.563	0	0	41.79 KB
#6614	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	697μs	3.85μs	23.1μs	0.329	0	0	41.83 KB
#6614	`WriteAndFlushEnrichedTraces`	net472	860μs	3.66μs	14.2μs	8.19	2.59	0.431	53.33 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`ExecuteNonQuery`	net6.0	1.27μs	1.45ns	5.61ns	0.0139	0	1.02 KB
master	`ExecuteNonQuery`	netcoreapp3.1	1.76μs	1.51ns	5.86ns	0.0134	0	1.02 KB
master	`ExecuteNonQuery`	net472	2.08μs	1.41ns	5.27ns	0.156	0.00105	987 B
#6614	`ExecuteNonQuery`	net6.0	1.37μs	1.96ns	7.34ns	0.0144	0	1.02 KB
#6614	`ExecuteNonQuery`	netcoreapp3.1	1.75μs	1.03ns	3.86ns	0.0131	0	1.02 KB
#6614	`ExecuteNonQuery`	net472	2.01μs	2.24ns	8.68ns	0.156	0.001	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.39ns	5.21ns	0.0137	976 B
master	`CallElasticsearch`	netcoreapp3.1	1.53μs	1.66ns	6.19ns	0.0129	976 B
master	`CallElasticsearch`	net472	2.56μs	1.67ns	6.46ns	0.157	995 B
master	`CallElasticsearchAsync`	net6.0	1.29μs	0.411ns	1.59ns	0.0135	952 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.74μs	1.73ns	6.48ns	0.0138	1.02 KB
master	`CallElasticsearchAsync`	net472	2.53μs	1.64ns	6.36ns	0.167	1.05 KB
#6614	`CallElasticsearch`	net6.0	1.22μs	0.789ns	2.95ns	0.0134	976 B
#6614	`CallElasticsearch`	netcoreapp3.1	1.54μs	3.15ns	11.8ns	0.0131	976 B
#6614	`CallElasticsearch`	net472	2.63μs	1.35ns	5.03ns	0.158	995 B
#6614	`CallElasticsearchAsync`	net6.0	1.27μs	0.926ns	3.59ns	0.0134	952 B
#6614	`CallElasticsearchAsync`	netcoreapp3.1	1.78μs	1.57ns	5.89ns	0.0133	1.02 KB
#6614	`CallElasticsearchAsync`	net472	2.54μs	2.06ns	8ns	0.167	1.05 KB

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

Faster 🎉 in #6614

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.GraphQLBenchmark.ExecuteAsync‑net6.0	1.122	1,429.56	1,273.91

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteAsync`	net6.0	1.43μs	0.743ns	2.88ns	0.0136	952 B
master	`ExecuteAsync`	netcoreapp3.1	1.68μs	0.575ns	2.15ns	0.0123	952 B
master	`ExecuteAsync`	net472	1.92μs	0.512ns	1.98ns	0.145	915 B
#6614	`ExecuteAsync`	net6.0	1.27μs	0.844ns	2.92ns	0.0133	952 B
#6614	`ExecuteAsync`	netcoreapp3.1	1.59μs	1.73ns	6.47ns	0.0129	952 B
#6614	`ExecuteAsync`	net472	1.92μs	0.688ns	2.48ns	0.144	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.46μs	23.2ns	109ns	0.0334	2.31 KB
master	`SendAsync`	netcoreapp3.1	5.38μs	1.54ns	5.57ns	0.0373	2.85 KB
master	`SendAsync`	net472	7.52μs	1.45ns	5.43ns	0.492	3.12 KB
#6614	`SendAsync`	net6.0	4.45μs	0.878ns	3.29ns	0.031	2.31 KB
#6614	`SendAsync`	netcoreapp3.1	5.28μs	0.883ns	3.06ns	0.0371	2.85 KB
#6614	`SendAsync`	net472	7.44μs	1.94ns	7.52ns	0.494	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.62μs	0.698ns	2.61ns	0.0227	1.64 KB
master	`EnrichedLog`	netcoreapp3.1	2.3μs	1.48ns	5.55ns	0.0218	1.64 KB
master	`EnrichedLog`	net472	2.51μs	2.1ns	8.15ns	0.249	1.57 KB
#6614	`EnrichedLog`	net6.0	1.48μs	1.03ns	3.98ns	0.0232	1.64 KB
#6614	`EnrichedLog`	netcoreapp3.1	2.36μs	1.38ns	5.33ns	0.0225	1.64 KB
#6614	`EnrichedLog`	net472	2.45μs	0.814ns	3.05ns	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	114μs	208ns	807ns	0	0	4.28 KB
master	`EnrichedLog`	netcoreapp3.1	120μs	178ns	688ns	0	0	4.28 KB
master	`EnrichedLog`	net472	150μs	125ns	483ns	0.673	0.224	4.46 KB
#6614	`EnrichedLog`	net6.0	113μs	162ns	605ns	0.0566	0	4.28 KB
#6614	`EnrichedLog`	netcoreapp3.1	117μs	168ns	629ns	0.0584	0	4.28 KB
#6614	`EnrichedLog`	net472	150μs	114ns	440ns	0.672	0.224	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.1μs	0.573ns	2.07ns	0.0311	2.2 KB
master	`EnrichedLog`	netcoreapp3.1	4.21μs	1.92ns	6.92ns	0.0295	2.2 KB
master	`EnrichedLog`	net472	4.86μs	0.927ns	3.59ns	0.32	2.02 KB
#6614	`EnrichedLog`	net6.0	2.99μs	1.35ns	5.24ns	0.0312	2.2 KB
#6614	`EnrichedLog`	netcoreapp3.1	4.22μs	1.38ns	5.15ns	0.0296	2.2 KB
#6614	`EnrichedLog`	net472	4.94μs	0.571ns	2.14ns	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.39μs	0.78ns	3.02ns	0.016	1.14 KB
master	`SendReceive`	netcoreapp3.1	1.72μs	2.24ns	8.68ns	0.0154	1.14 KB
master	`SendReceive`	net472	2.14μs	0.547ns	2.05ns	0.183	1.16 KB
#6614	`SendReceive`	net6.0	1.34μs	0.747ns	2.89ns	0.016	1.14 KB
#6614	`SendReceive`	netcoreapp3.1	1.79μs	1.24ns	4.8ns	0.0152	1.14 KB
#6614	`SendReceive`	net472	2.02μs	0.817ns	3.06ns	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.7μs	0.975ns	3.52ns	0.0216	1.6 KB
master	`EnrichedLog`	netcoreapp3.1	3.88μs	1.72ns	6.65ns	0.0213	1.65 KB
master	`EnrichedLog`	net472	4.25μs	4.09ns	15.8ns	0.323	2.04 KB
#6614	`EnrichedLog`	net6.0	2.71μs	3.02ns	11.7ns	0.0228	1.6 KB
#6614	`EnrichedLog`	netcoreapp3.1	3.77μs	1.73ns	6.69ns	0.0224	1.65 KB
#6614	`EnrichedLog`	net472	4.28μs	2.71ns	10.5ns	0.323	2.04 KB

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

Faster 🎉 in #6614

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑netcoreapp3.1	1.132	616.69	544.91

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net6.0	417ns	0.557ns	2.08ns	0.00809	576 B
master	`StartFinishSpan`	netcoreapp3.1	618ns	1.02ns	3.94ns	0.00772	576 B
master	`StartFinishSpan`	net472	671ns	1.18ns	4.58ns	0.0915	578 B
master	`StartFinishScope`	net6.0	472ns	0.831ns	3.22ns	0.00987	696 B
master	`StartFinishScope`	netcoreapp3.1	718ns	0.697ns	2.61ns	0.00924	696 B
master	`StartFinishScope`	net472	824ns	1.61ns	6.02ns	0.104	658 B
#6614	`StartFinishSpan`	net6.0	396ns	0.7ns	2.71ns	0.00808	576 B
#6614	`StartFinishSpan`	netcoreapp3.1	544ns	0.687ns	2.66ns	0.00771	576 B
#6614	`StartFinishSpan`	net472	696ns	1.71ns	6.64ns	0.0916	578 B
#6614	`StartFinishScope`	net6.0	479ns	0.844ns	3.27ns	0.0097	696 B
#6614	`StartFinishScope`	netcoreapp3.1	696ns	0.957ns	3.58ns	0.00913	696 B
#6614	`StartFinishScope`	net472	800ns	2.26ns	8.76ns	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	650ns	0.834ns	3.23ns	0.00961	696 B
master	`RunOnMethodBegin`	netcoreapp3.1	922ns	2.04ns	7.91ns	0.00939	696 B
master	`RunOnMethodBegin`	net472	1.05μs	2.66ns	10.3ns	0.104	658 B
#6614	`RunOnMethodBegin`	net6.0	634ns	1.29ns	4.99ns	0.00972	696 B
#6614	`RunOnMethodBegin`	netcoreapp3.1	943ns	1.39ns	5.4ns	0.00949	696 B
#6614	`RunOnMethodBegin`	net472	1.03μs	2.71ns	10.5ns	0.105	658 B

andrewlock · 2025-02-03T15:27:41Z

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 (6614) - mean (69ms)  : 66, 72
     .   : milestone, 69,
    master - mean (69ms)  : 66, 71
     .   : milestone, 69,

    section CallTarget+Inlining+NGEN
    This PR (6614) - mean (999ms)  : 980, 1017
     .   : milestone, 999,
    master - mean (998ms)  : 977, 1018
     .   : milestone, 998,

gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6614) - mean (102ms)  : 100, 104
     .   : milestone, 102,
    master - mean (102ms)  : 100, 104
     .   : milestone, 102,

    section CallTarget+Inlining+NGEN
    This PR (6614) - mean (671ms)  : 649, 692
     .   : milestone, 671,
    master - mean (673ms)  : 656, 690
     .   : milestone, 673,

gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6614) - mean (89ms)  : 88, 91
     .   : milestone, 89,
    master - mean (89ms)  : 87, 91
     .   : milestone, 89,

    section CallTarget+Inlining+NGEN
    This PR (6614) - mean (629ms)  : 610, 648
     .   : milestone, 629,
    master - mean (634ms)  : 618, 651
     .   : milestone, 634,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6614) - mean (191ms)  : 186, 195
     .   : milestone, 191,
    master - mean (191ms)  : 186, 196
     .   : milestone, 191,

    section CallTarget+Inlining+NGEN
    This PR (6614) - mean (1,105ms)  : 1080, 1129
     .   : milestone, 1105,
    master - mean (1,110ms)  : 1078, 1142
     .   : milestone, 1110,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6614) - mean (270ms)  : 266, 275
     .   : milestone, 270,
    master - mean (274ms)  : 267, 282
     .   : milestone, 274,

    section CallTarget+Inlining+NGEN
    This PR (6614) - mean (865ms)  : 827, 902
     .   : milestone, 865,
    master - mean (869ms)  : 829, 908
     .   : milestone, 869,

gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6614) - mean (262ms)  : 259, 265
     .   : milestone, 262,
    master - mean (262ms)  : 258, 266
     .   : milestone, 262,

    section CallTarget+Inlining+NGEN
    This PR (6614) - mean (843ms)  : 804, 883
     .   : milestone, 843,
    master - mean (848ms)  : 817, 880
     .   : milestone, 848,

github-actions · 2025-02-13T14:10:30Z

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.

1 occurrences of :

-      "hash": -543813396,
+      "hash": 799617955,
[...]
-        "path": "Samples.Security.AspNetCore5.Controllers.IastController",
-        "method": "SendMailAux"
+        "path": "Samples.Security.AspNetCore5.Helpers.EmailHelper",
+        "method": "SendEmailSystemLib"

anna-git

nice work! a few comments and nits

anna-git · 2025-02-19T09:43:43Z

...mples.InstrumentedTests/Vulnerabilities/EmailHtmlInjection/EmailHtmlInjectionMailKitTests.cs

+    }
+
+    [Fact]
+    public void GivenAnEmail_WhenSendHtmlMailMessageTaintedSanitizedValuesHtml_ThenIsNotVulnerable2()


maybe the similar ones could be theories?

There are some tainting issues when dealing with tainted data and memberdata. I tried to refactor the tests to make them use Theory and I got it, but it was pretty complex and not very readable. I have used Theory for encoding, though. Thanks!

anna-git · 2025-02-19T10:07:11Z

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

+                body = sendEmailRequest?.Message?.Body?.Html?.Data;
+                isHtml = !string.IsNullOrEmpty(body);
+                break;
+            default:


are we not checking MailKit? if not, it will fall to default and log an error, do we still want to consider it as an error, or a debug?

We will not fall into default because we instrument Mailkit differently and we don't call this method when using Mailkit. The problem about mailkit is that we loose the tainting after setting the body text, so we cannot instrument the Send methods.

anna-git · 2025-02-19T10:38:43Z

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

+                break;
+            case EmailInjectionType.AmazonSimpleEmail:
+                var sendEmailRequest = mail.DuckCast<ISendEmailRequest>();
+                body = sendEmailRequest?.Message?.Body?.Html?.Data;


Suggested change

body = sendEmailRequest?.Message?.Body?.Html?.Data;

body = sendEmailRequest.Message.Body.Html.Data;

iiuc, seems like we don't need all theses checks, as object mail is not nullable and if instance is not null return type is not null when duck casting per https://github.com/DataDog/dd-trace-dotnet/blob/master/docs/development/DuckTyping.md#2-using-interface-proxies-in-duckcastt-or-in-duck-chained-properties

Yes, you are right. I will remove the nullability check in mail? Still, Message might be null, so I will keep those nullability checks. I have updated the code. Thanks!

anna-git · 2025-02-19T10:54:16Z

tracer/src/Datadog.Trace/Iast/Aspects/MailKit/MailkitAspect.cs

+        IMimeKitTextPart? textPart = null;
+        try
+        {
+            textPart = instance.DuckCast<IMimeKitTextPart>();


should we use TryDuckCast instead?

We are following the same pattern in other aspects such as JsonDocumentAspects or JavaScriptSerializerAspects. I guess that we should probably log an DuckCast Failure/exception as error. WDYT?

oh ok, makes sense to log, I just thought TryDuckCast might have been more performant, and then testing the bool result to log or not :idk

it says:

If you know that your proxy is correct and that it will sometimes fail (due to the nature of the integration) you should use TryDuckCast over DuckAs to safely try the proxying and to handle failure. If you don't expect the proxying to fail (i.e. you're not accounting for explicitly known scenarios) then favour DuckCast

so I don't know if you expect the proxy to fail, maybe not so it's ok like this 🤔

Yes, it should not fail. I have tested different versions of the assembly and the proxy should not change.

anna-git · 2025-02-19T10:58:05Z

...rumentedTests/Vulnerabilities/EmailHtmlInjection/EmailHtmlInjectionAWSSDKSimpleEmailTests.cs

+    }
+
+    [Fact]
+    public void GivenAnEmail_WhenSendAsyncHtmlMailMessageTaintedVaulesHtmlEscaped_ThenIsNotVulnerable2()


same here, wondering if it could be factored as theories

Done! Thanks!

…om/DataDog/dd-trace-dotnet into nacho/emailInejctionAdditionalLibs

NachoEchevarria · 2025-02-25T13:14:36Z

Thanks for you feedback and reviews!

NachoEchevarria and others added 2 commits January 31, 2025 16:10

Email injection sample code

36732ad

Merge branch 'master' into nacho/emailInejctionAdditionalLibs

4d092f5

github-actions bot added the area:tests unit tests, integration tests label Jan 31, 2025

Unit tests

d958242

All unit tests

4c5aa8a

NachoEchevarria added 6 commits February 7, 2025 18:33

mailKit aspect

16c3d5a

MailKit tests passing

ecdd6cd

Amazon tests

9f4600a

New amazon tests.

e372ad1

Minor fixes

acf242b

Minor changess

a1f796a

NachoEchevarria changed the title ~~Email injection new libs~~ [ASM] Email injection: MimeKit and SimpleEmail Feb 13, 2025

Merge branch 'master' into nacho/emailInejctionAdditionalLibs

0db2d9e

github-actions bot added the area:asm label Feb 13, 2025

NachoEchevarria added 2 commits February 13, 2025 12:28

Fix merge issues

b978cdf

fix integration test

6ceded9

Merge branch 'master' into nacho/emailInejctionAdditionalLibs

144102b

DataDog deleted a comment from datadog-ddstaging bot Feb 17, 2025

NachoEchevarria marked this pull request as ready for review February 17, 2025 14:00

NachoEchevarria requested review from a team as code owners February 17, 2025 14:00

anna-git reviewed Feb 19, 2025

View reviewed changes

NachoEchevarria and others added 4 commits February 19, 2025 14:57

Remove nullable checks.

3e35506

Merge branch 'nacho/emailInejctionAdditionalLibs' of https://github.c…

47d8a99

…om/DataDog/dd-trace-dotnet into nacho/emailInejctionAdditionalLibs

Use theory in test

7e955a4

Merge branch 'master' into nacho/emailInejctionAdditionalLibs

2212a41

anna-git approved these changes Feb 21, 2025

View reviewed changes

NachoEchevarria added 2 commits February 24, 2025 11:59

Use theory in AWS tests

772455a

Merge branch 'nacho/emailInejctionAdditionalLibs' of https://github.c…

6ca9b88

…om/DataDog/dd-trace-dotnet into nacho/emailInejctionAdditionalLibs

lucaspimentel approved these changes Feb 24, 2025

View reviewed changes

NachoEchevarria merged commit 373bd64 into master Feb 25, 2025
139 of 150 checks passed

NachoEchevarria deleted the nacho/emailInejctionAdditionalLibs branch February 25, 2025 13:14

github-actions bot added this to the vNext-v3 milestone Feb 25, 2025

andrewlock added type:new-feature type:enhancement Improvement to an existing feature and removed type:new-feature labels Mar 11, 2025

	body = sendEmailRequest?.Message?.Body?.Html?.Data;
	body = sendEmailRequest.Message.Body.Html.Data;

[ASM] Email injection: MimeKit and SimpleEmail #6614

[ASM] Email injection: MimeKit and SimpleEmail #6614

Conversation

NachoEchevarria commented Jan 31, 2025 • edited Loading

Summary of changes

Reason for change

Implementation details

Test coverage

Other details

andrewlock commented Jan 31, 2025 • edited Loading

Benchmarks Report for appsec 🐌

Benchmark details

Slower ⚠️ in #6614

Faster 🎉 in #6614

Raw results

Raw results

Raw results

More allocations ⚠️ in #6614

Raw results

andrewlock commented Jan 31, 2025 • edited Loading

Benchmarks Report for tracer 🐌

Benchmark details

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Faster 🎉 in #6614

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Faster 🎉 in #6614

Raw results

Raw results

andrewlock commented Feb 3, 2025 • edited Loading

Execution-Time Benchmarks Report ⏱️

github-actions bot commented Feb 13, 2025

Snapshots difference summary

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NachoEchevarria commented Feb 25, 2025

NachoEchevarria commented Jan 31, 2025 •

edited

Loading

andrewlock commented Jan 31, 2025 •

edited

Loading

andrewlock commented Jan 31, 2025 •

edited

Loading

andrewlock commented Feb 3, 2025 •

edited

Loading