From 2112c9ad5280ef11f6c22518df9ab9373d59b3e6 Mon Sep 17 00:00:00 2001 From: Flavien Darche Date: Sun, 17 Nov 2024 17:09:07 +0100 Subject: [PATCH 01/16] aspects not working --- .../DefaultInterpolatedStringHandlerAspect.cs | 44 +++++++++++++++++++ .../String/DefaultInterpolatedStringTests.cs | 29 ++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs create mode 100644 tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringTests.cs diff --git a/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs b/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs new file mode 100644 index 000000000000..96555e8d9a72 --- /dev/null +++ b/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs @@ -0,0 +1,44 @@ +// +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. +// + +#if NET6_0_OR_GREATER + +using System; +using System.Runtime.CompilerServices; +using Datadog.Trace.Iast.Dataflow; + +namespace Datadog.Trace.Iast.Aspects.System.Runtime; + +#pragma warning disable DD0004 + +/// DefaultInterpolatedString class aspect +[AspectClass("System.Runtime")] +[global::System.ComponentModel.Browsable(false)] +[global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)] +public class DefaultInterpolatedStringHandlerAspect +{ + /// + /// System.Reflection Assembly.Load aspects + /// + /// target + /// value + /// DefaultInterpolatedStringHandler + [AspectMethodInsertBefore("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.String)", 0)] + public static DefaultInterpolatedStringHandler AppendFormatted(DefaultInterpolatedStringHandler target, string value) + { + try + { + target.AppendFormatted(value); + } + catch (Exception ex) + { + IastModule.Log.Error(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted)}"); + } + + return target; + } +} + +#endif diff --git a/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringTests.cs b/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringTests.cs new file mode 100644 index 000000000000..b5d8cd82fa78 --- /dev/null +++ b/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringTests.cs @@ -0,0 +1,29 @@ +#if NET6_0_OR_GREATER + +using Xunit; +using System.Runtime.CompilerServices; + +namespace Samples.InstrumentedTests.Iast.Vulnerabilities.StringPropagation; + +public class DefaultInterpolatedStringTests : InstrumentationTestsBase +{ + protected string taintedValue = "tainted"; + + public DefaultInterpolatedStringTests() + { + AddTainted(taintedValue); + } + + [Fact] + public void GivenAnInterpolatedString_WhenInterpolatingString_GetString_Vulnerable() + { + var test = new DefaultInterpolatedStringHandler(); + test.AppendFormatted("Hello"); + test.AppendFormatted(taintedValue); + + var str = test.ToStringAndClear(); + AssertTainted(str); + } +} + +#endif From ce53aaf81c8979d6de8220c2d6d91ad77a4e7e67 Mon Sep 17 00:00:00 2001 From: Flavien Darche Date: Tue, 19 Nov 2024 15:29:47 +0100 Subject: [PATCH 02/16] update --- .../DefaultInterpolatedStringHandlerAspect.cs | 50 +++++++++++++++-- ...ultInterpolatedStringHandlerIntegration.cs | 56 +++++++++++++++++++ ...aultInterpolatedStringHandlerModuleImpl.cs | 19 +++++++ .../String/DefaultInterpolatedStringTests.cs | 2 +- 4 files changed, 120 insertions(+), 7 deletions(-) create mode 100644 tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerIntegration.cs create mode 100644 tracer/src/Datadog.Trace/Iast/Propagation/DefaultInterpolatedStringHandlerModuleImpl.cs diff --git a/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs b/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs index 96555e8d9a72..a888336f40fc 100644 --- a/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs +++ b/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs @@ -11,7 +11,9 @@ namespace Datadog.Trace.Iast.Aspects.System.Runtime; -#pragma warning disable DD0004 +#pragma warning disable DD0005 +#pragma warning disable SA1642 +#pragma warning disable SA1107 /// DefaultInterpolatedString class aspect [AspectClass("System.Runtime")] @@ -24,20 +26,56 @@ public class DefaultInterpolatedStringHandlerAspect /// /// target /// value - /// DefaultInterpolatedStringHandler - [AspectMethodInsertBefore("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.String)", 0)] - public static DefaultInterpolatedStringHandler AppendFormatted(DefaultInterpolatedStringHandler target, string value) + [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.String)", 0)] + public static void AppendFormatted(ref DefaultInterpolatedStringHandler target, string value) { + target.AppendFormatted(value); + try { - target.AppendFormatted(value); + Console.WriteLine("AppendFormatted: " + value); } catch (Exception ex) { IastModule.Log.Error(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted)}"); } + } + + /// ctor aspect + /// Init target + /// Init string + /// Init string2 + [AspectCtorReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::.ctor(System.Int32,System.Int32)")] + public static void Init(ref InterpolatedStringHandlerWrapper target, int value, int value2) + { + // Crashing the process + target = new InterpolatedStringHandlerWrapper(); + } + + /// InterpolatedStringHandlerWrapper ctor aspect + public ref struct InterpolatedStringHandlerWrapper + { + private DefaultInterpolatedStringHandler _handler; - return target; + /// Test + public Span Test; + + /// + /// Initializes a new instance of the struct. + /// + /// h + public InterpolatedStringHandlerWrapper(DefaultInterpolatedStringHandler handler) + { + _handler = handler; + Test = new Span("lol".ToCharArray()); + } + + /// AppendFormatted + /// value + public void AppendFormatted(string value) + { + _handler.AppendFormatted(value); + } } } diff --git a/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerIntegration.cs b/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerIntegration.cs new file mode 100644 index 000000000000..4237290cb82d --- /dev/null +++ b/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerIntegration.cs @@ -0,0 +1,56 @@ +// +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. +// + +#if NET6_0_OR_GREATER + +#nullable enable + +using System.ComponentModel; +using Datadog.Trace.ClrProfiler; +using Datadog.Trace.ClrProfiler.AutoInstrumentation.MongoDb; +using Datadog.Trace.ClrProfiler.CallTarget; +using Datadog.Trace.Configuration; +/* +namespace Datadog.Trace.Iast.Aspects.System.Runtime +{ + /// + /// MongoDB.Driver.Core.WireProtocol.IWireProtocol<TResult> instrumentation + /// + [InstrumentMethod( + AssemblyName = "System.Runtime", + IntegrationName = nameof(IntegrationId.SystemRandom), + MinimumVersion = MongoDbIntegration.Major2Minor1, + MaximumVersion = "9.*.*", + MethodName = "AppendFormatted", + ParameterTypeNames = new[] { ClrNames.String }, + ReturnTypeName = ClrNames.Void, + TypeName = "System.Runtime.CompilerServices.DefaultInterpolatedStringHandler")] + [InstrumentMethod( + AssemblyName = "System.Runtime", + IntegrationName = nameof(IntegrationId.SystemRandom), + MinimumVersion = MongoDbIntegration.Major2Minor1, + MaximumVersion = "9.*.*", + MethodName = "AppendFormatted", + ParameterTypeNames = new[] { ClrNames.String }, + ReturnTypeName = ClrNames.Void, + TypeName = "System.Runtime.CompilerServices.DefaultInterpolatedStringHandler&")] + [Browsable(false)] + [EditorBrowsable(EditorBrowsableState.Never)] + public class DefaultInterpolatedStringHandlerIntegration + { + /// + /// OnMethodBegin callback + /// + /// Instance value, aka `this` of the instrumented method. + /// value + /// Calltarget state value + internal static CallTargetState OnMethodBegin(TTarget instance, string value) + { + return CallTargetState.GetDefault(); + } + } +} +*/ +#endif diff --git a/tracer/src/Datadog.Trace/Iast/Propagation/DefaultInterpolatedStringHandlerModuleImpl.cs b/tracer/src/Datadog.Trace/Iast/Propagation/DefaultInterpolatedStringHandlerModuleImpl.cs new file mode 100644 index 000000000000..a37309954e54 --- /dev/null +++ b/tracer/src/Datadog.Trace/Iast/Propagation/DefaultInterpolatedStringHandlerModuleImpl.cs @@ -0,0 +1,19 @@ +// +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. +// + +#if NET6_0_OR_GREATER + +using System.Runtime.CompilerServices; + +namespace Datadog.Trace.Iast.Propagation; + +internal static class DefaultInterpolatedStringHandlerModuleImpl +{ + public static void AppendFormatted(DefaultInterpolatedStringHandler target, string value) + { + } +} + +#endif diff --git a/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringTests.cs b/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringTests.cs index b5d8cd82fa78..2bc14feaa1ce 100644 --- a/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringTests.cs +++ b/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringTests.cs @@ -17,7 +17,7 @@ public DefaultInterpolatedStringTests() [Fact] public void GivenAnInterpolatedString_WhenInterpolatingString_GetString_Vulnerable() { - var test = new DefaultInterpolatedStringHandler(); + var test = new DefaultInterpolatedStringHandler(1,1); test.AppendFormatted("Hello"); test.AppendFormatted(taintedValue); From 32defa88bc7b769dbfc59c917bfc5d8a45319045 Mon Sep 17 00:00:00 2001 From: Daniel Romano <108014683+daniel-romano-DD@users.noreply.github.com> Date: Thu, 21 Nov 2024 23:38:58 +0100 Subject: [PATCH 03/16] Preliminary support for ref struct instrumentation to cover interpolated strings --- tracer/missing-nullability-files.csv | 1 + .../DefaultInterpolatedStringHandlerAspect.cs | 54 ++++----- .../Datadog.Trace/Iast/DefaultTaintedMap.cs | 15 ++- ...aultInterpolatedStringHandlerModuleImpl.cs | 105 +++++++++++++++++- .../iast/dataflow_aspects.cpp | 34 +++++- .../iast/dataflow_aspects.h | 7 +- .../String/DefaultInterpolatedStringTests.cs | 20 +++- .../iast.runsettings | 16 ++- 8 files changed, 205 insertions(+), 47 deletions(-) diff --git a/tracer/missing-nullability-files.csv b/tracer/missing-nullability-files.csv index c3e91339768f..7f018e0148a8 100644 --- a/tracer/missing-nullability-files.csv +++ b/tracer/missing-nullability-files.csv @@ -578,6 +578,7 @@ src/Datadog.Trace/Debugger/Sink/Models/ProbeStatus.cs src/Datadog.Trace/Debugger/Sink/Models/Status.cs src/Datadog.Trace/Debugger/Sink/Models/TimedMessage.cs src/Datadog.Trace/Iast/Aspects/System.DirectoryServices/SearchRequestAspect.cs +src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs src/Datadog.Trace/Iast/Aspects/System.Web.SessionState/HttpSessionStateBaseAspect.cs src/Datadog.Trace/Iast/Aspects/System.Web.SessionState/SessionExtensionsAspect.cs src/Datadog.Trace/Iast/Aspects/System/StringAspects.cs diff --git a/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs b/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs index a888336f40fc..b35a0ee65e1b 100644 --- a/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs +++ b/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs @@ -8,6 +8,7 @@ using System; using System.Runtime.CompilerServices; using Datadog.Trace.Iast.Dataflow; +using Datadog.Trace.Iast.Propagation; namespace Datadog.Trace.Iast.Aspects.System.Runtime; @@ -25,15 +26,15 @@ public class DefaultInterpolatedStringHandlerAspect /// System.Reflection Assembly.Load aspects /// /// target + /// target pointer /// value - [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.String)", 0)] - public static void AppendFormatted(ref DefaultInterpolatedStringHandler target, string value) + [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.String)")] + public static void AppendFormatted(ref DefaultInterpolatedStringHandler target, IntPtr targetPtr, string value) { target.AppendFormatted(value); - try { - Console.WriteLine("AppendFormatted: " + value); + DefaultInterpolatedStringHandlerModuleImpl.AppendFormatted(targetPtr, value); } catch (Exception ex) { @@ -41,41 +42,26 @@ public static void AppendFormatted(ref DefaultInterpolatedStringHandler target, } } - /// ctor aspect - /// Init target - /// Init string - /// Init string2 - [AspectCtorReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::.ctor(System.Int32,System.Int32)")] - public static void Init(ref InterpolatedStringHandlerWrapper target, int value, int value2) - { - // Crashing the process - target = new InterpolatedStringHandlerWrapper(); - } - - /// InterpolatedStringHandlerWrapper ctor aspect - public ref struct InterpolatedStringHandlerWrapper + /// + /// System.Reflection Assembly.Load aspects + /// + /// target + /// target pointer + /// string value + [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::ToStringAndClear()")] + public static string ToStringAndClear(ref DefaultInterpolatedStringHandler target, IntPtr targetPtr) { - private DefaultInterpolatedStringHandler _handler; - - /// Test - public Span Test; - - /// - /// Initializes a new instance of the struct. - /// - /// h - public InterpolatedStringHandlerWrapper(DefaultInterpolatedStringHandler handler) + var result = target.ToStringAndClear(); + try { - _handler = handler; - Test = new Span("lol".ToCharArray()); + DefaultInterpolatedStringHandlerModuleImpl.PropagateTaint(targetPtr, result); } - - /// AppendFormatted - /// value - public void AppendFormatted(string value) + catch (Exception ex) { - _handler.AppendFormatted(value); + IastModule.Log.Error(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted)}"); } + + return result; } } diff --git a/tracer/src/Datadog.Trace/Iast/DefaultTaintedMap.cs b/tracer/src/Datadog.Trace/Iast/DefaultTaintedMap.cs index 2b3969c58754..7c75ec2866e0 100644 --- a/tracer/src/Datadog.Trace/Iast/DefaultTaintedMap.cs +++ b/tracer/src/Datadog.Trace/Iast/DefaultTaintedMap.cs @@ -62,12 +62,23 @@ public DefaultTaintedMap() } _map.TryGetValue(IndexObject(objectToFind), out var entry); + bool isString = objectToFind is string; while (entry != null) { - if (objectToFind == entry.Value) + if (isString) { - return entry; + if (objectToFind == entry.Value) + { + return entry; + } + } + else + { + if (objectToFind.Equals(entry.Value)) + { + return entry; + } } entry = entry.Next; diff --git a/tracer/src/Datadog.Trace/Iast/Propagation/DefaultInterpolatedStringHandlerModuleImpl.cs b/tracer/src/Datadog.Trace/Iast/Propagation/DefaultInterpolatedStringHandlerModuleImpl.cs index a37309954e54..3406fb0d962b 100644 --- a/tracer/src/Datadog.Trace/Iast/Propagation/DefaultInterpolatedStringHandlerModuleImpl.cs +++ b/tracer/src/Datadog.Trace/Iast/Propagation/DefaultInterpolatedStringHandlerModuleImpl.cs @@ -5,14 +5,117 @@ #if NET6_0_OR_GREATER +#nullable enable + +using System; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using System.Text; +using Datadog.Trace.VendoredMicrosoftCode.System.Runtime.InteropServices; namespace Datadog.Trace.Iast.Propagation; internal static class DefaultInterpolatedStringHandlerModuleImpl { - public static void AppendFormatted(DefaultInterpolatedStringHandler target, string value) + public static unsafe void AppendFormatted(IntPtr target, string value) + { + FullTaintIfAnyTainted(target, value); + } + + public static unsafe void FullTaintIfAnyTainted(IntPtr target, object? firstInput = null, object? secondInput = null, object? thirdInput = null, object? fourthInput = null) + { + try + { + IastModule.OnExecutedPropagationTelemetry(); + + var iastContext = IastModule.GetIastContext(); + if (iastContext is null) + { + return; + } + + var taintedObjects = iastContext.GetTaintedObjects(); + var tainted = PropagationModuleImpl.GetTainted(taintedObjects, target); + bool targetIsTainted = tainted is not null; + + if (!targetIsTainted) + { + if (((tainted = GetTaintedWithRanges(taintedObjects, firstInput)) is null) && + ((tainted = GetTaintedWithRanges(taintedObjects, secondInput)) is null) && + ((tainted = GetTaintedWithRanges(taintedObjects, thirdInput)) is null) && + ((tainted = GetTaintedWithRanges(taintedObjects, fourthInput)) is null)) + { + return; + } + } + + var rangesResult = new Range[] { new Range(0, -1, tainted!.Ranges[0].Source) }; + if (!targetIsTainted) + { + taintedObjects.Taint(target, rangesResult); + } + else + { + tainted.Ranges = rangesResult; + } + } + catch (Exception error) + { + IastModule.Log.Error(error, $"{nameof(StringBuilderModuleImpl)}.{nameof(FullTaintIfAnyTainted)} exception"); + } + } + + public static object? PropagateTaint(object? input, object? result, int offset = 0, bool addTelemetry = true) + { + try + { + if (addTelemetry) + { + IastModule.OnExecutedPropagationTelemetry(); + } + + if (result is null || input is null) + { + return result; + } + + var iastContext = IastModule.GetIastContext(); + if (iastContext == null) + { + return result; + } + + var taintedObjects = iastContext.GetTaintedObjects(); + var taintedSelf = taintedObjects.Get(input); + + if (taintedSelf == null) + { + return result; + } + + if (offset != 0) + { + var newRanges = new Range[taintedSelf.Ranges.Length]; + Ranges.CopyShift(taintedSelf.Ranges, newRanges, 0, offset); + taintedObjects.Taint(result, newRanges); + } + else + { + taintedObjects.Taint(result, taintedSelf.Ranges); + } + } + catch (Exception err) + { + IastModule.Log.Error(err, "PropagationModuleImpl.PropagateTaint exception"); + } + + return result; + } + + private static TaintedObject? GetTaintedWithRanges(TaintedObjects taintedObjects, object? value) { + var tainted = PropagationModuleImpl.GetTainted(taintedObjects, value); + return tainted is not null && tainted?.Ranges.Length > 0 ? tainted : null; } } diff --git a/tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.cpp b/tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.cpp index e54c7214a922..36e334f7e7e1 100644 --- a/tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.cpp +++ b/tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.cpp @@ -111,6 +111,9 @@ namespace iast } } + // TODO : For POC only + _isRefStruct = _aspectTypeName == WStr("Datadog.Trace.Iast.Aspects.System.Runtime.DefaultInterpolatedStringHandlerAspect"); + _isValid = true; } @@ -119,6 +122,11 @@ namespace iast return _isValid; } + bool DataflowAspectClass::IsRefStruct() + { + return _isRefStruct; + } + bool DataflowAspectClass::IsTargetModule(ModuleInfo* module) { return Contains(_assemblies, module->_name); @@ -271,6 +279,7 @@ namespace iast mdTypeRef targetMethodTypeRef = 0; mdMemberRef targetMethodRef = 0; mdTypeRef targetTypeRef = 0; + bool isRefStruct = false; std::vector paramTypeRefs; std::vector targetMethodRefCandidates; hr = module->FindTypeRefByName(_targetMethodType.c_str(), &targetMethodTypeRef); @@ -340,7 +349,7 @@ namespace iast if (targetMethodRef != 0) { - return new DataflowAspectReference(moduleAspects, this, targetMethodRef, targetTypeRef, paramTypeRefs); + return new DataflowAspectReference(moduleAspects, this, targetMethodRef, targetTypeRef, isRefStruct, paramTypeRefs); } } return nullptr; @@ -353,7 +362,7 @@ namespace iast //------------------------------ - DataflowAspectReference::DataflowAspectReference(ModuleAspects* moduleAspects, DataflowAspect* aspect, mdMemberRef method, mdTypeRef type, const std::vector& paramType) + DataflowAspectReference::DataflowAspectReference(ModuleAspects* moduleAspects, DataflowAspect* aspect, mdMemberRef method, mdTypeRef type, bool isRefStruct, const std::vector& paramType) { this->_moduleAspects = moduleAspects; this->_module = moduleAspects->_module; @@ -361,6 +370,7 @@ namespace iast this->_targetMethodRef = method; this->_targetTypeRef = type; this->_targetParamTypeToken = paramType; + this->_isRefStruct = isRefStruct; if (this->_targetParamTypeToken.size() == 0) { this->_targetParamTypeToken.push_back(0); } std::vector filterValues; @@ -574,7 +584,7 @@ namespace iast context.instruction = inserted; } } - else //Replace + else // Replace { if (_aspect->_paramShift.size() > 0) { @@ -585,7 +595,8 @@ namespace iast continue; } - auto instructionInfo = processor->StackAnalysis()->GetInstruction(instructionToProcess.instruction); + auto instructionInfo = + processor->StackAnalysis()->GetInstruction(instructionToProcess.instruction); auto methodSig = instructionInfo->GetArgumentSignature(); int paramCount = methodSig->GetEffectiveParamCount(); for (auto iInfo : processor->StackAnalysis()->LocateCallParamInstructions( @@ -593,7 +604,8 @@ namespace iast paramCount - _aspect->_paramShift[x] - 1)) // Locate param load instruction { auto paramType = _targetParamTypeToken[x]; - processor->InsertAfter(iInfo->_instruction, processor->NewILInstr(CEE_BOX, paramType, true)); + processor->InsertAfter(iInfo->_instruction, + processor->NewILInstr(CEE_BOX, paramType, true)); // Figure out if param is byref if (iInfo->IsArgument()) @@ -602,7 +614,8 @@ namespace iast auto param = sig->_params[iInfo->_instruction->m_Arg32]; if (param->IsByRef()) { - processor->InsertAfter(iInfo->_instruction, processor->NewILInstr(CEE_LDOBJ, paramType, true)); + processor->InsertAfter(iInfo->_instruction, + processor->NewILInstr(CEE_LDOBJ, paramType, true)); } } @@ -611,6 +624,15 @@ namespace iast } } + if (_aspect->_aspectClass->IsRefStruct()) + { + // Duplicate first param as IntPtr + auto firstParam = processor->StackAnalysis()->LocateCallParamInstructions(instruction, 0).front(); + processor->InsertAfter(firstParam->_instruction, + processor->NewILInstr(CEE_DUP, 0, true)); + + } + aspectInstruction = instructionToProcess.instruction; instructionToProcess.instruction->m_opcode = CEE_CALL; instructionToProcess.instruction->m_Arg32 = memberRef; diff --git a/tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.h b/tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.h index 372ebd523ff3..dd1031544431 100644 --- a/tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.h +++ b/tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.h @@ -41,7 +41,8 @@ namespace iast protected: WSTRING _line = EmptyWStr; bool _isValid = false; - mdTypeDef _aspectTypeDef = 0; + bool _isRefStruct = false; + public: Dataflow* _dataflow; WSTRING _aspectsAssembly; @@ -54,6 +55,7 @@ namespace iast std::vector _filters; bool IsValid(); + bool IsRefStruct(); bool IsTargetModule(ModuleInfo* module); WSTRING ToString(); @@ -122,7 +124,7 @@ namespace iast class DataflowAspectReference { public: - DataflowAspectReference(ModuleAspects* moduleAspects, DataflowAspect* aspect, mdMemberRef targetMethod, mdTypeRef targetType, const std::vector& paramType); + DataflowAspectReference(ModuleAspects* moduleAspects, DataflowAspect* aspect, mdMemberRef targetMethod, mdTypeRef targetType, bool isRefStruct, const std::vector& paramType); public: DataflowAspect* _aspect = nullptr; @@ -130,6 +132,7 @@ namespace iast ModuleInfo* _module = nullptr; mdMemberRef _targetMethodRef = 0; mdTypeRef _targetTypeRef = 0; + bool _isRefStruct = false; std::vector _targetParamTypeToken; std::vector _filters; diff --git a/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringTests.cs b/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringTests.cs index 2bc14feaa1ce..7ece65a00dff 100644 --- a/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringTests.cs +++ b/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringTests.cs @@ -8,6 +8,7 @@ namespace Samples.InstrumentedTests.Iast.Vulnerabilities.StringPropagation; public class DefaultInterpolatedStringTests : InstrumentationTestsBase { protected string taintedValue = "tainted"; + protected string untaintedValue = "untainted"; public DefaultInterpolatedStringTests() { @@ -15,7 +16,7 @@ public DefaultInterpolatedStringTests() } [Fact] - public void GivenAnInterpolatedString_WhenInterpolatingString_GetString_Vulnerable() + public void GivenAnExplicitInterpolatedString_WhenAddingTaintedValue_GetString_Vulnerable() { var test = new DefaultInterpolatedStringHandler(1,1); test.AppendFormatted("Hello"); @@ -24,6 +25,23 @@ public void GivenAnInterpolatedString_WhenInterpolatingString_GetString_Vulnerab var str = test.ToStringAndClear(); AssertTainted(str); } + + [Fact] + public void GivenAnImplicitInterpolatedString_WhenAddingTaintedValue_GetString_Vulnerable() + { + var number = 5; + var str = $"Hello {taintedValue} {number}"; + AssertTainted(str); + } + + [Fact] + public void GivenAnImplicitInterpolatedString_WhenAddingUntaintedValue_GetString_NonVulnerable() + { + var number = 5; + var str = $"Hello {untaintedValue} {number}"; + AssertNotTainted(str); + } + } #endif diff --git a/tracer/test/test-applications/integrations/Samples.InstrumentedTests/iast.runsettings b/tracer/test/test-applications/integrations/Samples.InstrumentedTests/iast.runsettings index 6e28d1dfd49b..2a69176ce1a8 100644 --- a/tracer/test/test-applications/integrations/Samples.InstrumentedTests/iast.runsettings +++ b/tracer/test/test-applications/integrations/Samples.InstrumentedTests/iast.runsettings @@ -1,3 +1,17 @@ - \ No newline at end of file +1 +{846F5F1C-F9AE-4B07-969E-05C26BC060D8} +C:\_DD\Git\dd-trace-dotnet\tracer\test\test-applications\integrations\Samples.InstrumentedTests\..\..\..\..\..\shared\bin\monitoring-home\win-x64\Datadog.Trace.ClrProfiler.Native.dll +C:\_DD\Git\dd-trace-dotnet\tracer\test\test-applications\integrations\Samples.InstrumentedTests\..\..\..\..\..\shared\bin\monitoring-home\win-x86\Datadog.Trace.ClrProfiler.Native.dll +1 +{846F5F1C-F9AE-4B07-969E-05C26BC060D8} +C:\_DD\Git\dd-trace-dotnet\tracer\test\test-applications\integrations\Samples.InstrumentedTests\..\..\..\..\..\shared\bin\monitoring-home\win-x64\Datadog.Trace.ClrProfiler.Native.dll +C:\_DD\Git\dd-trace-dotnet\tracer\test\test-applications\integrations\Samples.InstrumentedTests\..\..\..\..\..\shared\bin\monitoring-home\win-x86\Datadog.Trace.ClrProfiler.Native.dll +C:\_DD\Git\dd-trace-dotnet\tracer\test\test-applications\integrations\Samples.InstrumentedTests\..\..\..\..\..\shared\bin\monitoring-home +1.0.0 +1 +1 +0 +0 + From 23288bd18ce24df8e3be50a1c2115867d4b8e0a3 Mon Sep 17 00:00:00 2001 From: Daniel Romano <108014683+daniel-romano-DD@users.noreply.github.com> Date: Fri, 22 Nov 2024 09:42:05 +0100 Subject: [PATCH 04/16] Rebase fix --- .../Datadog.Tracer.Native/Generated/generated_callsites.g.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tracer/src/Datadog.Tracer.Native/Generated/generated_callsites.g.h b/tracer/src/Datadog.Tracer.Native/Generated/generated_callsites.g.h index cade04aec78d..c5f2491800fa 100644 --- a/tracer/src/Datadog.Tracer.Native/Generated/generated_callsites.g.h +++ b/tracer/src/Datadog.Tracer.Native/Generated/generated_callsites.g.h @@ -621,6 +621,9 @@ std::vector g_callSites= (WCHAR*)WStr("[AspectClass(\"System.Private.Corelib;System.Runtime\",[None],Sink,[Ssrf])] Datadog.Trace.Iast.Aspects.System.Net.WebUtilityAspect 4"), (WCHAR*)WStr(" [AspectMethodReplace(\"System.Net.WebUtility::HtmlEncode(System.String)\",\"\",[0],[False],[None],Default,[])] XssEscape(System.String) 165"), (WCHAR*)WStr(" [AspectMethodReplace(\"System.Net.WebUtility::UrlEncode(System.String)\",\"\",[0],[False],[None],Default,[])] SsrfEscape(System.String) 165"), +(WCHAR*)WStr("[AspectClass(\"System.Runtime\",[None],Propagation,[])] Datadog.Trace.Iast.Aspects.System.Runtime.DefaultInterpolatedStringHandlerAspect 4"), +(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.String)\",\"\",[0],[False],[None],Default,[])] AppendFormatted(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.IntPtr,System.String) 128"), +(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::ToStringAndClear()\",\"\",[0],[False],[None],Default,[])] ToStringAndClear(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.IntPtr) 128"), (WCHAR*)WStr("[AspectClass(\"System.Text.Json\",[None],Source,[])] Datadog.Trace.Iast.Aspects.System.Text.Json.JsonDocumentAspects 4"), (WCHAR*)WStr(" [AspectMethodReplace(\"System.Text.Json.JsonDocument::Parse(System.String,System.Text.Json.JsonDocumentOptions)\",\"\",[0],[False],[None],Default,[]);V2.49.0] Parse(System.String,System.Text.Json.JsonDocumentOptions) 160"), (WCHAR*)WStr(" [AspectMethodReplace(\"System.Text.Json.JsonElement::GetRawText()\",\"\",[0],[True],[None],Default,[]);V2.49.0] GetRawText(System.Object) 160"), From 826f2022c44377931e9fc2d9e678036df12bd95e Mon Sep 17 00:00:00 2001 From: Daniel Romano <108014683+daniel-romano-DD@users.noreply.github.com> Date: Fri, 22 Nov 2024 09:46:31 +0100 Subject: [PATCH 05/16] Rebase fix --- .../Samples.InstrumentedTests/iast.runsettings | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/tracer/test/test-applications/integrations/Samples.InstrumentedTests/iast.runsettings b/tracer/test/test-applications/integrations/Samples.InstrumentedTests/iast.runsettings index 2a69176ce1a8..6e28d1dfd49b 100644 --- a/tracer/test/test-applications/integrations/Samples.InstrumentedTests/iast.runsettings +++ b/tracer/test/test-applications/integrations/Samples.InstrumentedTests/iast.runsettings @@ -1,17 +1,3 @@ -1 -{846F5F1C-F9AE-4B07-969E-05C26BC060D8} -C:\_DD\Git\dd-trace-dotnet\tracer\test\test-applications\integrations\Samples.InstrumentedTests\..\..\..\..\..\shared\bin\monitoring-home\win-x64\Datadog.Trace.ClrProfiler.Native.dll -C:\_DD\Git\dd-trace-dotnet\tracer\test\test-applications\integrations\Samples.InstrumentedTests\..\..\..\..\..\shared\bin\monitoring-home\win-x86\Datadog.Trace.ClrProfiler.Native.dll -1 -{846F5F1C-F9AE-4B07-969E-05C26BC060D8} -C:\_DD\Git\dd-trace-dotnet\tracer\test\test-applications\integrations\Samples.InstrumentedTests\..\..\..\..\..\shared\bin\monitoring-home\win-x64\Datadog.Trace.ClrProfiler.Native.dll -C:\_DD\Git\dd-trace-dotnet\tracer\test\test-applications\integrations\Samples.InstrumentedTests\..\..\..\..\..\shared\bin\monitoring-home\win-x86\Datadog.Trace.ClrProfiler.Native.dll -C:\_DD\Git\dd-trace-dotnet\tracer\test\test-applications\integrations\Samples.InstrumentedTests\..\..\..\..\..\shared\bin\monitoring-home -1.0.0 -1 -1 -0 -0 - + \ No newline at end of file From 008717e25901e712028774519944bd183de700e0 Mon Sep 17 00:00:00 2001 From: Daniel Romano <108014683+daniel-romano-DD@users.noreply.github.com> Date: Fri, 22 Nov 2024 11:39:26 +0100 Subject: [PATCH 06/16] Converted dyn method from Emit to Fody --- tracer/missing-nullability-files.csv | 1 - .../DefaultInterpolatedStringHandlerAspect.cs | 21 +++++++++++++------ .../Generated/generated_callsites.g.h | 4 ++-- .../iast/dataflow_aspects.cpp | 17 --------------- .../iast/dataflow_aspects.h | 2 -- 5 files changed, 17 insertions(+), 28 deletions(-) diff --git a/tracer/missing-nullability-files.csv b/tracer/missing-nullability-files.csv index 7f018e0148a8..c3e91339768f 100644 --- a/tracer/missing-nullability-files.csv +++ b/tracer/missing-nullability-files.csv @@ -578,7 +578,6 @@ src/Datadog.Trace/Debugger/Sink/Models/ProbeStatus.cs src/Datadog.Trace/Debugger/Sink/Models/Status.cs src/Datadog.Trace/Debugger/Sink/Models/TimedMessage.cs src/Datadog.Trace/Iast/Aspects/System.DirectoryServices/SearchRequestAspect.cs -src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs src/Datadog.Trace/Iast/Aspects/System.Web.SessionState/HttpSessionStateBaseAspect.cs src/Datadog.Trace/Iast/Aspects/System.Web.SessionState/SessionExtensionsAspect.cs src/Datadog.Trace/Iast/Aspects/System/StringAspects.cs diff --git a/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs b/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs index b35a0ee65e1b..7b2607022fd1 100644 --- a/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs +++ b/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs @@ -6,9 +6,14 @@ #if NET6_0_OR_GREATER using System; +using System.Reflection.Emit; using System.Runtime.CompilerServices; using Datadog.Trace.Iast.Dataflow; using Datadog.Trace.Iast.Propagation; +using InlineIL; +using static InlineIL.IL.Emit; + +#nullable enable namespace Datadog.Trace.Iast.Aspects.System.Runtime; @@ -26,15 +31,14 @@ public class DefaultInterpolatedStringHandlerAspect /// System.Reflection Assembly.Load aspects /// /// target - /// target pointer /// value [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.String)")] - public static void AppendFormatted(ref DefaultInterpolatedStringHandler target, IntPtr targetPtr, string value) + public static void AppendFormatted(ref DefaultInterpolatedStringHandler target, string value) { target.AppendFormatted(value); try { - DefaultInterpolatedStringHandlerModuleImpl.AppendFormatted(targetPtr, value); + DefaultInterpolatedStringHandlerModuleImpl.AppendFormatted(ToPointer(ref target), value); } catch (Exception ex) { @@ -46,15 +50,14 @@ public static void AppendFormatted(ref DefaultInterpolatedStringHandler target, /// System.Reflection Assembly.Load aspects /// /// target - /// target pointer /// string value [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::ToStringAndClear()")] - public static string ToStringAndClear(ref DefaultInterpolatedStringHandler target, IntPtr targetPtr) + public static string ToStringAndClear(ref DefaultInterpolatedStringHandler target) { var result = target.ToStringAndClear(); try { - DefaultInterpolatedStringHandlerModuleImpl.PropagateTaint(targetPtr, result); + DefaultInterpolatedStringHandlerModuleImpl.PropagateTaint(ToPointer(ref target), result); } catch (Exception ex) { @@ -63,6 +66,12 @@ public static string ToStringAndClear(ref DefaultInterpolatedStringHandler targe return result; } + + private static unsafe IntPtr ToPointer(ref DefaultInterpolatedStringHandler ts) + { + Ldarg(nameof(ts)); + return IL.Return(); + } } #endif diff --git a/tracer/src/Datadog.Tracer.Native/Generated/generated_callsites.g.h b/tracer/src/Datadog.Tracer.Native/Generated/generated_callsites.g.h index c5f2491800fa..58b3a526d901 100644 --- a/tracer/src/Datadog.Tracer.Native/Generated/generated_callsites.g.h +++ b/tracer/src/Datadog.Tracer.Native/Generated/generated_callsites.g.h @@ -622,8 +622,8 @@ std::vector g_callSites= (WCHAR*)WStr(" [AspectMethodReplace(\"System.Net.WebUtility::HtmlEncode(System.String)\",\"\",[0],[False],[None],Default,[])] XssEscape(System.String) 165"), (WCHAR*)WStr(" [AspectMethodReplace(\"System.Net.WebUtility::UrlEncode(System.String)\",\"\",[0],[False],[None],Default,[])] SsrfEscape(System.String) 165"), (WCHAR*)WStr("[AspectClass(\"System.Runtime\",[None],Propagation,[])] Datadog.Trace.Iast.Aspects.System.Runtime.DefaultInterpolatedStringHandlerAspect 4"), -(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.String)\",\"\",[0],[False],[None],Default,[])] AppendFormatted(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.IntPtr,System.String) 128"), -(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::ToStringAndClear()\",\"\",[0],[False],[None],Default,[])] ToStringAndClear(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.IntPtr) 128"), +(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.String)\",\"\",[0],[False],[None],Default,[])] AppendFormatted(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.String) 128"), +(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::ToStringAndClear()\",\"\",[0],[False],[None],Default,[])] ToStringAndClear(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler) 128"), (WCHAR*)WStr("[AspectClass(\"System.Text.Json\",[None],Source,[])] Datadog.Trace.Iast.Aspects.System.Text.Json.JsonDocumentAspects 4"), (WCHAR*)WStr(" [AspectMethodReplace(\"System.Text.Json.JsonDocument::Parse(System.String,System.Text.Json.JsonDocumentOptions)\",\"\",[0],[False],[None],Default,[]);V2.49.0] Parse(System.String,System.Text.Json.JsonDocumentOptions) 160"), (WCHAR*)WStr(" [AspectMethodReplace(\"System.Text.Json.JsonElement::GetRawText()\",\"\",[0],[True],[None],Default,[]);V2.49.0] GetRawText(System.Object) 160"), diff --git a/tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.cpp b/tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.cpp index 36e334f7e7e1..8857c973cfde 100644 --- a/tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.cpp +++ b/tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.cpp @@ -111,9 +111,6 @@ namespace iast } } - // TODO : For POC only - _isRefStruct = _aspectTypeName == WStr("Datadog.Trace.Iast.Aspects.System.Runtime.DefaultInterpolatedStringHandlerAspect"); - _isValid = true; } @@ -122,11 +119,6 @@ namespace iast return _isValid; } - bool DataflowAspectClass::IsRefStruct() - { - return _isRefStruct; - } - bool DataflowAspectClass::IsTargetModule(ModuleInfo* module) { return Contains(_assemblies, module->_name); @@ -624,15 +616,6 @@ namespace iast } } - if (_aspect->_aspectClass->IsRefStruct()) - { - // Duplicate first param as IntPtr - auto firstParam = processor->StackAnalysis()->LocateCallParamInstructions(instruction, 0).front(); - processor->InsertAfter(firstParam->_instruction, - processor->NewILInstr(CEE_DUP, 0, true)); - - } - aspectInstruction = instructionToProcess.instruction; instructionToProcess.instruction->m_opcode = CEE_CALL; instructionToProcess.instruction->m_Arg32 = memberRef; diff --git a/tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.h b/tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.h index dd1031544431..15d5f0e07689 100644 --- a/tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.h +++ b/tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.h @@ -41,7 +41,6 @@ namespace iast protected: WSTRING _line = EmptyWStr; bool _isValid = false; - bool _isRefStruct = false; public: Dataflow* _dataflow; @@ -55,7 +54,6 @@ namespace iast std::vector _filters; bool IsValid(); - bool IsRefStruct(); bool IsTargetModule(ModuleInfo* module); WSTRING ToString(); From 41d581c214b35f9d6961fe926386ac970a5370f5 Mon Sep 17 00:00:00 2001 From: Flavien Darche Date: Fri, 22 Nov 2024 18:59:14 +0100 Subject: [PATCH 07/16] Add update aspects and test for detection sqli interpolated string --- .../DefaultInterpolatedStringHandlerAspect.cs | 108 +++++++++-- ...aultInterpolatedStringHandlerModuleImpl.cs | 18 +- .../Generated/generated_callsites.g.h | 6 +- .../IAST/AspNetCore5IastTests.cs | 23 +++ ...tring.AspNetCore5.IastEnabled.verified.txt | 1 + .../DefaultInterpolatedStringHandlerTests.cs | 177 ++++++++++++++++++ .../String/DefaultInterpolatedStringTests.cs | 47 ----- .../Controllers/IastController.cs | 42 +++++ 8 files changed, 347 insertions(+), 75 deletions(-) create mode 100644 tracer/test/snapshots/Iast.SqliInterpolatedString.AspNetCore5.IastEnabled.verified.txt create mode 100644 tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringHandlerTests.cs delete mode 100644 tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringTests.cs diff --git a/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs b/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs index 7b2607022fd1..c7ce264d28dd 100644 --- a/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs +++ b/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs @@ -17,10 +17,6 @@ namespace Datadog.Trace.Iast.Aspects.System.Runtime; -#pragma warning disable DD0005 -#pragma warning disable SA1642 -#pragma warning disable SA1107 - /// DefaultInterpolatedString class aspect [AspectClass("System.Runtime")] [global::System.ComponentModel.Browsable(false)] @@ -28,29 +24,111 @@ namespace Datadog.Trace.Iast.Aspects.System.Runtime; public class DefaultInterpolatedStringHandlerAspect { /// - /// System.Reflection Assembly.Load aspects + /// System.Runtime DefaultInterpolatedStringHandler.AppendFormatted(String) aspect /// - /// target - /// value + /// the ref DefaultInterpolatedStringHandler + /// the string value [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.String)")] - public static void AppendFormatted(ref DefaultInterpolatedStringHandler target, string value) + public static void AppendFormatted1(ref DefaultInterpolatedStringHandler target, string value) { target.AppendFormatted(value); try { - DefaultInterpolatedStringHandlerModuleImpl.AppendFormatted(ToPointer(ref target), value); + DefaultInterpolatedStringHandlerModuleImpl.Append(ToPointer(ref target), value); + } + catch (Exception ex) + { + IastModule.Log.Error(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted1)}"); + } + } + + /// + /// System.Runtime DefaultInterpolatedStringHandler.AppendFormatted(T, String) aspect + /// + /// the ref DefaultInterpolatedStringHandler + /// the string value + /// the alignment value + [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0,System.Int32)")] + public static void AppendFormatted2(ref DefaultInterpolatedStringHandler target, object value, int alignment) + { + target.AppendFormatted(value, alignment); + try + { + if (value is string str) + { + DefaultInterpolatedStringHandlerModuleImpl.Append(ToPointer(ref target), str); + } + } + catch (Exception ex) + { + IastModule.Log.Error(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted2)}"); + } + } + + /// + /// System.Runtime DefaultInterpolatedStringHandler.AppendFormatted(String, Int32, String) aspect + /// + /// the ref DefaultInterpolatedStringHandler + /// the string value + /// the alignment value + /// the format value + [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.String,System.Int32,System.String)")] + public static void AppendFormatted3(ref DefaultInterpolatedStringHandler target, string? value, int alignment, string? format) + { + target.AppendFormatted(value, alignment, format); + try + { + DefaultInterpolatedStringHandlerModuleImpl.Append(ToPointer(ref target), value); + } + catch (Exception ex) + { + IastModule.Log.Error(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted3)}"); + } + } + + /// + /// System.Runtime DefaultInterpolatedStringHandler.AppendFormatted(String, Int32, String) aspect + /// + /// the ref DefaultInterpolatedStringHandler + /// the string value + [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0)")] + public static void AppendFormatted4(ref DefaultInterpolatedStringHandler target, string value) + { + target.AppendFormatted(value); + try + { + DefaultInterpolatedStringHandlerModuleImpl.Append(ToPointer(ref target), value); + } + catch (Exception ex) + { + IastModule.Log.Error(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted4)}"); + } + } + + /// + /// System.Runtime DefaultInterpolatedStringHandler.AppendLiteral(String) aspect + /// + /// the ref DefaultInterpolatedStringHandler + /// the string value + [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendLiteral(System.String)")] + public static void AppendLiteral(ref DefaultInterpolatedStringHandler target, string value) + { + target.AppendLiteral(value); + try + { + DefaultInterpolatedStringHandlerModuleImpl.Append(ToPointer(ref target), value); } catch (Exception ex) { - IastModule.Log.Error(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted)}"); + IastModule.Log.Error(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendLiteral)}"); } } /// - /// System.Reflection Assembly.Load aspects + /// System.Runtime DefaultInterpolatedStringHandler.ToStringAndClear aspect /// - /// target - /// string value + /// the ref DefaultInterpolatedStringHandler + /// the string value [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::ToStringAndClear()")] public static string ToStringAndClear(ref DefaultInterpolatedStringHandler target) { @@ -61,13 +139,13 @@ public static string ToStringAndClear(ref DefaultInterpolatedStringHandler targe } catch (Exception ex) { - IastModule.Log.Error(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted)}"); + IastModule.Log.Error(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(ToStringAndClear)}"); } return result; } - private static unsafe IntPtr ToPointer(ref DefaultInterpolatedStringHandler ts) + private static IntPtr ToPointer(ref DefaultInterpolatedStringHandler ts) { Ldarg(nameof(ts)); return IL.Return(); diff --git a/tracer/src/Datadog.Trace/Iast/Propagation/DefaultInterpolatedStringHandlerModuleImpl.cs b/tracer/src/Datadog.Trace/Iast/Propagation/DefaultInterpolatedStringHandlerModuleImpl.cs index 3406fb0d962b..51d0c75789f9 100644 --- a/tracer/src/Datadog.Trace/Iast/Propagation/DefaultInterpolatedStringHandlerModuleImpl.cs +++ b/tracer/src/Datadog.Trace/Iast/Propagation/DefaultInterpolatedStringHandlerModuleImpl.cs @@ -17,12 +17,12 @@ namespace Datadog.Trace.Iast.Propagation; internal static class DefaultInterpolatedStringHandlerModuleImpl { - public static unsafe void AppendFormatted(IntPtr target, string value) + public static unsafe void Append(IntPtr target, string? value) { FullTaintIfAnyTainted(target, value); } - public static unsafe void FullTaintIfAnyTainted(IntPtr target, object? firstInput = null, object? secondInput = null, object? thirdInput = null, object? fourthInput = null) + public static unsafe void FullTaintIfAnyTainted(IntPtr target, object? input = null) { try { @@ -36,20 +36,14 @@ public static unsafe void FullTaintIfAnyTainted(IntPtr target, object? firstInpu var taintedObjects = iastContext.GetTaintedObjects(); var tainted = PropagationModuleImpl.GetTainted(taintedObjects, target); - bool targetIsTainted = tainted is not null; + var targetIsTainted = tainted is not null; - if (!targetIsTainted) + if (!targetIsTainted && (tainted = GetTaintedWithRanges(taintedObjects, input)) is null) { - if (((tainted = GetTaintedWithRanges(taintedObjects, firstInput)) is null) && - ((tainted = GetTaintedWithRanges(taintedObjects, secondInput)) is null) && - ((tainted = GetTaintedWithRanges(taintedObjects, thirdInput)) is null) && - ((tainted = GetTaintedWithRanges(taintedObjects, fourthInput)) is null)) - { - return; - } + return; } - var rangesResult = new Range[] { new Range(0, -1, tainted!.Ranges[0].Source) }; + var rangesResult = new[] { new Range(0, -1, tainted!.Ranges[0].Source) }; if (!targetIsTainted) { taintedObjects.Taint(target, rangesResult); diff --git a/tracer/src/Datadog.Tracer.Native/Generated/generated_callsites.g.h b/tracer/src/Datadog.Tracer.Native/Generated/generated_callsites.g.h index 58b3a526d901..473f528d6d37 100644 --- a/tracer/src/Datadog.Tracer.Native/Generated/generated_callsites.g.h +++ b/tracer/src/Datadog.Tracer.Native/Generated/generated_callsites.g.h @@ -622,7 +622,11 @@ std::vector g_callSites= (WCHAR*)WStr(" [AspectMethodReplace(\"System.Net.WebUtility::HtmlEncode(System.String)\",\"\",[0],[False],[None],Default,[])] XssEscape(System.String) 165"), (WCHAR*)WStr(" [AspectMethodReplace(\"System.Net.WebUtility::UrlEncode(System.String)\",\"\",[0],[False],[None],Default,[])] SsrfEscape(System.String) 165"), (WCHAR*)WStr("[AspectClass(\"System.Runtime\",[None],Propagation,[])] Datadog.Trace.Iast.Aspects.System.Runtime.DefaultInterpolatedStringHandlerAspect 4"), -(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.String)\",\"\",[0],[False],[None],Default,[])] AppendFormatted(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.String) 128"), +(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0,System.Int32)\",\"\",[0],[False],[None],Default,[])] AppendFormatted2(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.Object,System.Int32) 128"), +(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.String)\",\"\",[0],[False],[None],Default,[])] AppendFormatted1(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.String) 128"), +(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.String,System.Int32,System.String)\",\"\",[0],[False],[None],Default,[])] AppendFormatted3(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.String,System.Int32,System.String) 128"), +(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0)\",\"\",[0],[False],[None],Default,[])] AppendFormatted4(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.String) 128"), +(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendLiteral(System.String)\",\"\",[0],[False],[None],Default,[])] AppendLiteral(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.String) 128"), (WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::ToStringAndClear()\",\"\",[0],[False],[None],Default,[])] ToStringAndClear(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler) 128"), (WCHAR*)WStr("[AspectClass(\"System.Text.Json\",[None],Source,[])] Datadog.Trace.Iast.Aspects.System.Text.Json.JsonDocumentAspects 4"), (WCHAR*)WStr(" [AspectMethodReplace(\"System.Text.Json.JsonDocument::Parse(System.String,System.Text.Json.JsonDocumentOptions)\",\"\",[0],[False],[None],Default,[]);V2.49.0] Parse(System.String,System.Text.Json.JsonDocumentOptions) 160"), diff --git a/tracer/test/Datadog.Trace.Security.IntegrationTests/IAST/AspNetCore5IastTests.cs b/tracer/test/Datadog.Trace.Security.IntegrationTests/IAST/AspNetCore5IastTests.cs index df41591988ac..a5d988811516 100644 --- a/tracer/test/Datadog.Trace.Security.IntegrationTests/IAST/AspNetCore5IastTests.cs +++ b/tracer/test/Datadog.Trace.Security.IntegrationTests/IAST/AspNetCore5IastTests.cs @@ -382,6 +382,29 @@ await VerifyHelper.VerifySpans(spansFiltered, settings) .UseFileName(filename) .DisableRequireUniquePrefix(); } + + #if NET6_0_OR_GREATER + [SkippableFact] + [Trait("Category", "ArmUnsupported")] + [Trait("RunOnWindows", "True")] + public async Task TestIastSqliInterpolatedString() + { + var filename = "Iast.SqliInterpolatedString.AspNetCore5." + (IastEnabled ? "IastEnabled" : "IastDisabled"); + if (RedactionEnabled is true) { filename += ".RedactionEnabled"; } + var url = $"/Iast/InterpolatedSqlString?name=John"; + IncludeAllHttpSpans = true; + await TryStartApp(); + var agent = Fixture.Agent; + var spans = await SendRequestsAsync(agent, 2, new string[] { url }); + var spansFiltered = spans.Where(x => x.Type == SpanTypes.Web || x.Type == SpanTypes.IastVulnerability).ToList(); + + var settings = VerifyHelper.GetSpanVerifierSettings(); + settings.AddIastScrubbing(); + await VerifyHelper.VerifySpans(spansFiltered, settings) + .UseFileName(filename) + .DisableRequireUniquePrefix(); + } + #endif } // Classes to test particular features diff --git a/tracer/test/snapshots/Iast.SqliInterpolatedString.AspNetCore5.IastEnabled.verified.txt b/tracer/test/snapshots/Iast.SqliInterpolatedString.AspNetCore5.IastEnabled.verified.txt new file mode 100644 index 000000000000..5f282702bb03 --- /dev/null +++ b/tracer/test/snapshots/Iast.SqliInterpolatedString.AspNetCore5.IastEnabled.verified.txt @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringHandlerTests.cs b/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringHandlerTests.cs new file mode 100644 index 000000000000..b1506713ad7d --- /dev/null +++ b/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringHandlerTests.cs @@ -0,0 +1,177 @@ +#if NET6_0_OR_GREATER + +using System; +using Xunit; +using System.Runtime.CompilerServices; + +namespace Samples.InstrumentedTests.Iast.Vulnerabilities.StringPropagation; + +public class DefaultInterpolatedStringHandlerTests : InstrumentationTestsBase +{ + protected string TaintedValue = "tainted"; + protected string UntaintedValue = "untainted"; + + public DefaultInterpolatedStringHandlerTests() + { + AddTainted(TaintedValue); + } + + [Fact] + public void GivenAnExplicitInterpolatedString_WhenAddingTaintedValueAppendFormatted1_GetString_Vulnerable() + { + var test = new DefaultInterpolatedStringHandler(); + test.AppendFormatted(TaintedValue); + + AssertTainted(test.ToStringAndClear()); + } + + [Fact] + public void GivenAnExplicitInterpolatedString_WhenAddingUntaintedValueAppendFormatted1_GetString_NonVulnerable() + { + var test = new DefaultInterpolatedStringHandler(); + test.AppendFormatted(UntaintedValue); + + AssertNotTainted(test.ToStringAndClear()); + } + + [Fact] + public void GivenAnExplicitInterpolatedString_WhenAddingTaintedValueAppendFormatted2_GetString_Vulnerable() + { + var test = new DefaultInterpolatedStringHandler(); + test.AppendFormatted(TaintedValue, 0); + + AssertTainted(test.ToStringAndClear()); + } + + [Fact] + public void GivenAnExplicitInterpolatedString_WhenAddingUntaintedValueAppendFormatted2_GetString_NonVulnerable() + { + var test = new DefaultInterpolatedStringHandler(); + test.AppendFormatted(UntaintedValue, 0); + + AssertNotTainted(test.ToStringAndClear()); + } + + [Fact] + public void GivenAnExplicitInterpolatedString_WhenAddingTaintedValueAppendFormatted3_GetString_Vulnerable() + { + var test = new DefaultInterpolatedStringHandler(); + test.AppendFormatted(TaintedValue, 0, null); + + AssertTainted(test.ToStringAndClear()); + } + + [Fact] + public void GivenAnExplicitInterpolatedString_WhenAddingUntaintedValueAppendFormatted3_GetString_NonVulnerable() + { + var test = new DefaultInterpolatedStringHandler(); + test.AppendFormatted(UntaintedValue, 0, null); + + AssertNotTainted(test.ToStringAndClear()); + } + + [Fact] + public void GivenAnExplicitInterpolatedString_WhenAddingTaintedValueAppendLiteral_GetString_Vulnerable() + { + var test = new DefaultInterpolatedStringHandler(); + test.AppendLiteral(TaintedValue); + + AssertTainted(test.ToStringAndClear()); + } + + [Fact] + public void GivenAnExplicitInterpolatedString_WhenAddingUntaintedValueAppendLiteral_GetString_NonVulnerable() + { + var test = new DefaultInterpolatedStringHandler(); + test.AppendFormatted(UntaintedValue); + + AssertNotTainted(test.ToStringAndClear()); + } + + [Fact] + public void GivenAnExplicitInterpolatedString_WhenAddingTaintedValueMultipleValues_GetString_Vulnerable() + { + var test = new DefaultInterpolatedStringHandler(); + test.AppendLiteral(UntaintedValue); + test.AppendFormatted(new ReadOnlySpan([' ', 'w', 'o', 'r', 'l', 'd', ' '])); + test.AppendFormatted(TaintedValue); + test.AppendFormatted(42); + + AssertTainted(test.ToStringAndClear()); + } + + [Fact] + public void GivenAnImplicitInterpolatedString_WhenAddingTaintedValue_GetString_Vulnerable() + { + var number = 5; + var str = $"Hello {TaintedValue} {number}"; + AssertTainted(str); + } + + [Fact] + public void GivenAnImplicitInterpolatedString_WhenAddingUntaintedValue_GetString_NonVulnerable() + { + var number = 5; + var str = $"Hello {UntaintedValue} {number}"; + AssertNotTainted(str); + } + + [Fact] + public void GivenAnImplicitInterpolatedString_WhenAddingMultipleValuesWithTaintedValues_GetString_Vulnerable() + { + var order = new + { + CustomerId = "VINET", + EmployeeId = 5, + OrderDate = new DateTime(2021, 1, 1), + RequiredDate = new DateTime(2021, 1, 1), + ShipVia = 3, + Freight = 32.38M, + ShipName = "Vins et alcools Chevalier", + ShipAddress = TaintedValue, + ShipCity = "Reims", + ShipPostalCode = "51100", + ShipCountry = "France" + }; + + var sql = "INSERT INTO Orders (" + + "CustomerId, EmployeeId, OrderDate, RequiredDate, ShipVia, Freight, ShipName, ShipAddress, " + + "ShipCity, ShipPostalCode, ShipCountry" + + ") VALUES (" + + $"'{order.CustomerId}','{order.EmployeeId}','{order.OrderDate:yyyy-MM-dd}','{order.RequiredDate:yyyy-MM-dd}'," + + $"'{order.ShipVia}','{order.Freight}','{order.ShipName}','{order.ShipAddress}'," + + $"'{order.ShipCity}','{order.ShipPostalCode}','{order.ShipCountry}')"; + + AssertTainted(sql); + } + + [Fact] + public void GivenImplicitInterpolatedString_WhenAddingTaintedValuesNested_GetString_Vulnerable() + { + const int number = 42; + var date = new DateTime(2024, 11, 22, 15, 30, 0); + const decimal decimalValue = 123.456m; + const bool booleanValue = true; + const char charValue = 'A'; + var nestedInterpolatedString1 = $"Nested1 {TaintedValue} and {number}"; + var nestedInterpolatedString2 = $"Nested2 {nestedInterpolatedString1} with date {date:yyyy-MM-dd}"; + var complexString = $"Complex {nestedInterpolatedString2} and decimal {decimalValue:F2} and boolean {booleanValue}"; + + var nestedString = $"Hello {$"{TaintedValue + "Hello"} - {complexString}"}"; + var finalString = $"Final {nestedString} and char {charValue} with additional {UntaintedValue} and number {number} and date {date:HH:mm:ss}"; + + AssertTainted(finalString); + } + + [Fact] + public void GivenImplicitInterpolatedString_WhenAddingTaintedValuesComplex_GetString_Vulnerable() + { + var interpolatedString = $""" + Hello "{TaintedValue}" and "{UntaintedValue}". + . + """; + AssertTainted(interpolatedString); + } +} + +#endif diff --git a/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringTests.cs b/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringTests.cs deleted file mode 100644 index 7ece65a00dff..000000000000 --- a/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringTests.cs +++ /dev/null @@ -1,47 +0,0 @@ -#if NET6_0_OR_GREATER - -using Xunit; -using System.Runtime.CompilerServices; - -namespace Samples.InstrumentedTests.Iast.Vulnerabilities.StringPropagation; - -public class DefaultInterpolatedStringTests : InstrumentationTestsBase -{ - protected string taintedValue = "tainted"; - protected string untaintedValue = "untainted"; - - public DefaultInterpolatedStringTests() - { - AddTainted(taintedValue); - } - - [Fact] - public void GivenAnExplicitInterpolatedString_WhenAddingTaintedValue_GetString_Vulnerable() - { - var test = new DefaultInterpolatedStringHandler(1,1); - test.AppendFormatted("Hello"); - test.AppendFormatted(taintedValue); - - var str = test.ToStringAndClear(); - AssertTainted(str); - } - - [Fact] - public void GivenAnImplicitInterpolatedString_WhenAddingTaintedValue_GetString_Vulnerable() - { - var number = 5; - var str = $"Hello {taintedValue} {number}"; - AssertTainted(str); - } - - [Fact] - public void GivenAnImplicitInterpolatedString_WhenAddingUntaintedValue_GetString_NonVulnerable() - { - var number = 5; - var str = $"Hello {untaintedValue} {number}"; - AssertNotTainted(str); - } - -} - -#endif diff --git a/tracer/test/test-applications/security/Samples.Security.AspNetCore5/Controllers/IastController.cs b/tracer/test/test-applications/security/Samples.Security.AspNetCore5/Controllers/IastController.cs index 7fc23f9ebfd2..9616d8cd66e0 100644 --- a/tracer/test/test-applications/security/Samples.Security.AspNetCore5/Controllers/IastController.cs +++ b/tracer/test/test-applications/security/Samples.Security.AspNetCore5/Controllers/IastController.cs @@ -1065,6 +1065,48 @@ private static string GetDbValue(IDbConnection db, string name = "Name1") var res = reader.GetString(0); return res; } + + #if NET6_0_OR_GREATER + [HttpGet("InterpolatedSqlString")] + [Route("InterpolatedSqlString")] + public IActionResult InterpolatedSqlString(string name) + { + var order = new + { + CustomerId = "VINET", + EmployeeId = 5, + OrderDate = new DateTime(2021, 1, 1), + RequiredDate = new DateTime(2021, 1, 1), + ShipVia = 3, + Freight = 32.38M, + ShipName = "Vins et alcools Chevalier", + ShipAddress = name, + ShipCity = "Reims", + ShipPostalCode = "51100", + ShipCountry = "France" + }; + + var sql = "INSERT INTO Orders (" + + "CustomerId, EmployeeId, OrderDate, RequiredDate, ShipVia, Freight, ShipName, ShipAddress, " + + "ShipCity, ShipPostalCode, ShipCountry" + + ") VALUES (" + + $"'{order.CustomerId}','{order.EmployeeId}','{order.OrderDate:yyyy-MM-dd}','{order.RequiredDate:yyyy-MM-dd}'," + + $"'{order.ShipVia}','{order.Freight}','{order.ShipName}','{order.ShipAddress}'," + + $"'{order.ShipCity}','{order.ShipPostalCode}','{order.ShipCountry}')"; + sql += ";\nSELECT OrderID FROM Orders ORDER BY OrderID DESC LIMIT 1;"; + + try + { + new SqliteCommand(sql, DbConnectionSystemDataMicrosoftData).ExecuteScalar(); + } + catch (Exception ex) + { + // ignored + } + + return Content("Yey"); + } + #endif [HttpGet("TestJsonTagSizeExceeded")] [Route("TestJsonTagSizeExceeded")] From ca238ad778f257e59aeea89e6f9b1bbb7c0720cc Mon Sep 17 00:00:00 2001 From: Flavien Darche Date: Sun, 24 Nov 2024 19:19:04 +0100 Subject: [PATCH 08/16] update to good snapshot --- ...aultInterpolatedStringHandlerModuleImpl.cs | 32 +++---- ...tring.AspNetCore5.IastEnabled.verified.txt | 94 ++++++++++++++++++- 2 files changed, 106 insertions(+), 20 deletions(-) diff --git a/tracer/src/Datadog.Trace/Iast/Propagation/DefaultInterpolatedStringHandlerModuleImpl.cs b/tracer/src/Datadog.Trace/Iast/Propagation/DefaultInterpolatedStringHandlerModuleImpl.cs index 51d0c75789f9..64fe026b8910 100644 --- a/tracer/src/Datadog.Trace/Iast/Propagation/DefaultInterpolatedStringHandlerModuleImpl.cs +++ b/tracer/src/Datadog.Trace/Iast/Propagation/DefaultInterpolatedStringHandlerModuleImpl.cs @@ -22,12 +22,17 @@ public static unsafe void Append(IntPtr target, string? value) FullTaintIfAnyTainted(target, value); } - public static unsafe void FullTaintIfAnyTainted(IntPtr target, object? input = null) + public static unsafe void FullTaintIfAnyTainted(IntPtr target, string? input) { try { IastModule.OnExecutedPropagationTelemetry(); + if (input is null) + { + return; + } + var iastContext = IastModule.GetIastContext(); if (iastContext is null) { @@ -43,7 +48,8 @@ public static unsafe void FullTaintIfAnyTainted(IntPtr target, object? input = n return; } - var rangesResult = new[] { new Range(0, -1, tainted!.Ranges[0].Source) }; + // The start and length here are not correct, but we don't currently have a way to get the correct values + var rangesResult = new[] { new Range(0, 0, tainted!.Ranges[0].Source) }; if (!targetIsTainted) { taintedObjects.Taint(target, rangesResult); @@ -55,18 +61,15 @@ public static unsafe void FullTaintIfAnyTainted(IntPtr target, object? input = n } catch (Exception error) { - IastModule.Log.Error(error, $"{nameof(StringBuilderModuleImpl)}.{nameof(FullTaintIfAnyTainted)} exception"); + IastModule.Log.Error(error, $"{nameof(DefaultInterpolatedStringHandlerModuleImpl)}.{nameof(FullTaintIfAnyTainted)} exception"); } } - public static object? PropagateTaint(object? input, object? result, int offset = 0, bool addTelemetry = true) + public static object? PropagateTaint(object? input, object? result) { try { - if (addTelemetry) - { - IastModule.OnExecutedPropagationTelemetry(); - } + IastModule.OnExecutedPropagationTelemetry(); if (result is null || input is null) { @@ -87,20 +90,11 @@ public static unsafe void FullTaintIfAnyTainted(IntPtr target, object? input = n return result; } - if (offset != 0) - { - var newRanges = new Range[taintedSelf.Ranges.Length]; - Ranges.CopyShift(taintedSelf.Ranges, newRanges, 0, offset); - taintedObjects.Taint(result, newRanges); - } - else - { - taintedObjects.Taint(result, taintedSelf.Ranges); - } + taintedObjects.Taint(result, taintedSelf.Ranges); } catch (Exception err) { - IastModule.Log.Error(err, "PropagationModuleImpl.PropagateTaint exception"); + IastModule.Log.Error(err, $"{nameof(DefaultInterpolatedStringHandlerModuleImpl)}.{nameof(PropagateTaint)} exception"); } return result; diff --git a/tracer/test/snapshots/Iast.SqliInterpolatedString.AspNetCore5.IastEnabled.verified.txt b/tracer/test/snapshots/Iast.SqliInterpolatedString.AspNetCore5.IastEnabled.verified.txt index 5f282702bb03..2dcde2ce3397 100644 --- a/tracer/test/snapshots/Iast.SqliInterpolatedString.AspNetCore5.IastEnabled.verified.txt +++ b/tracer/test/snapshots/Iast.SqliInterpolatedString.AspNetCore5.IastEnabled.verified.txt @@ -1 +1,93 @@ - \ No newline at end of file +[ + { + TraceId: Id_1, + SpanId: Id_2, + Name: aspnet_core.request, + Resource: GET /iast/interpolatedsqlstring, + Service: Samples.Security.AspNetCore5, + Type: web, + Tags: { + aspnet_core.endpoint: Samples.Security.AspNetCore5.Controllers.IastController.InterpolatedSqlString (Samples.Security.AspNetCore5), + aspnet_core.route: iast/interpolatedsqlstring, + component: aspnet_core, + env: integration_tests, + http.method: GET, + http.request.headers.host: localhost:00000, + http.route: iast/interpolatedsqlstring, + http.status_code: 200, + http.url: http://localhost:00000/Iast/InterpolatedSqlString?name=John, + http.useragent: Mistake Not..., + language: dotnet, + runtime-id: Guid_1, + span.kind: server, + _dd.iast.enabled: 1, + _dd.iast.json: +{ + "vulnerabilities": [ + { + "type": "SQL_INJECTION", + "hash": -41956447, + "location": { + "spanId": XXX, + "path": "Samples.Security.AspNetCore5.Controllers.IastController", + "method": "InterpolatedSqlString", + "line": XXX + }, + "evidence": { + "valueParts": [ + { + "value": "INSERT INTO Orders (CustomerId, EmployeeId, OrderDate, RequiredDate, ShipVia, Freight, ShipName, ShipAddress, ShipCity, ShipPostalCode, ShipCountry) VALUES ('VINET','5','2021-01-01','2021-01-01'," + }, + { + "value": "", + "source": 0 + }, + { + "value": "'3','32,38','Vins et alcools Chevalier','John'," + }, + { + "value": "", + "source": 0 + }, + { + "value": "'Reims','51100','France');\nSELECT OrderID FROM Orders ORDER BY OrderID DESC LIMIT 1;" + } + ] + } + } + ], + "sources": [ + { + "origin": "http.request.parameter", + "name": "name", + "value": "John" + } + ] +} + }, + Metrics: { + process_id: 0, + _dd.top_level: 1.0, + _dd.tracer_kr: 1.0, + _sampling_priority_v1: 2.0 + } + }, + { + TraceId: Id_1, + SpanId: Id_3, + Name: aspnet_core_mvc.request, + Resource: GET /iast/interpolatedsqlstring, + Service: Samples.Security.AspNetCore5, + Type: web, + ParentId: Id_2, + Tags: { + aspnet_core.action: interpolatedsqlstring, + aspnet_core.controller: iast, + aspnet_core.route: iast/interpolatedsqlstring, + component: aspnet_core, + env: integration_tests, + language: dotnet, + span.kind: server + } + } +] \ No newline at end of file From 8b65a315f32e2c91fadb3286025b19c0a76d1bdf Mon Sep 17 00:00:00 2001 From: Flavien Darche Date: Sun, 24 Nov 2024 19:24:23 +0100 Subject: [PATCH 09/16] remove useless + revert changes in native part --- ...ultInterpolatedStringHandlerIntegration.cs | 56 ------------------- .../iast/dataflow_aspects.cpp | 17 ++---- .../iast/dataflow_aspects.h | 5 +- 3 files changed, 8 insertions(+), 70 deletions(-) delete mode 100644 tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerIntegration.cs diff --git a/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerIntegration.cs b/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerIntegration.cs deleted file mode 100644 index 4237290cb82d..000000000000 --- a/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerIntegration.cs +++ /dev/null @@ -1,56 +0,0 @@ -// -// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License. -// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. -// - -#if NET6_0_OR_GREATER - -#nullable enable - -using System.ComponentModel; -using Datadog.Trace.ClrProfiler; -using Datadog.Trace.ClrProfiler.AutoInstrumentation.MongoDb; -using Datadog.Trace.ClrProfiler.CallTarget; -using Datadog.Trace.Configuration; -/* -namespace Datadog.Trace.Iast.Aspects.System.Runtime -{ - /// - /// MongoDB.Driver.Core.WireProtocol.IWireProtocol<TResult> instrumentation - /// - [InstrumentMethod( - AssemblyName = "System.Runtime", - IntegrationName = nameof(IntegrationId.SystemRandom), - MinimumVersion = MongoDbIntegration.Major2Minor1, - MaximumVersion = "9.*.*", - MethodName = "AppendFormatted", - ParameterTypeNames = new[] { ClrNames.String }, - ReturnTypeName = ClrNames.Void, - TypeName = "System.Runtime.CompilerServices.DefaultInterpolatedStringHandler")] - [InstrumentMethod( - AssemblyName = "System.Runtime", - IntegrationName = nameof(IntegrationId.SystemRandom), - MinimumVersion = MongoDbIntegration.Major2Minor1, - MaximumVersion = "9.*.*", - MethodName = "AppendFormatted", - ParameterTypeNames = new[] { ClrNames.String }, - ReturnTypeName = ClrNames.Void, - TypeName = "System.Runtime.CompilerServices.DefaultInterpolatedStringHandler&")] - [Browsable(false)] - [EditorBrowsable(EditorBrowsableState.Never)] - public class DefaultInterpolatedStringHandlerIntegration - { - /// - /// OnMethodBegin callback - /// - /// Instance value, aka `this` of the instrumented method. - /// value - /// Calltarget state value - internal static CallTargetState OnMethodBegin(TTarget instance, string value) - { - return CallTargetState.GetDefault(); - } - } -} -*/ -#endif diff --git a/tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.cpp b/tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.cpp index 8857c973cfde..e54c7214a922 100644 --- a/tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.cpp +++ b/tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.cpp @@ -271,7 +271,6 @@ namespace iast mdTypeRef targetMethodTypeRef = 0; mdMemberRef targetMethodRef = 0; mdTypeRef targetTypeRef = 0; - bool isRefStruct = false; std::vector paramTypeRefs; std::vector targetMethodRefCandidates; hr = module->FindTypeRefByName(_targetMethodType.c_str(), &targetMethodTypeRef); @@ -341,7 +340,7 @@ namespace iast if (targetMethodRef != 0) { - return new DataflowAspectReference(moduleAspects, this, targetMethodRef, targetTypeRef, isRefStruct, paramTypeRefs); + return new DataflowAspectReference(moduleAspects, this, targetMethodRef, targetTypeRef, paramTypeRefs); } } return nullptr; @@ -354,7 +353,7 @@ namespace iast //------------------------------ - DataflowAspectReference::DataflowAspectReference(ModuleAspects* moduleAspects, DataflowAspect* aspect, mdMemberRef method, mdTypeRef type, bool isRefStruct, const std::vector& paramType) + DataflowAspectReference::DataflowAspectReference(ModuleAspects* moduleAspects, DataflowAspect* aspect, mdMemberRef method, mdTypeRef type, const std::vector& paramType) { this->_moduleAspects = moduleAspects; this->_module = moduleAspects->_module; @@ -362,7 +361,6 @@ namespace iast this->_targetMethodRef = method; this->_targetTypeRef = type; this->_targetParamTypeToken = paramType; - this->_isRefStruct = isRefStruct; if (this->_targetParamTypeToken.size() == 0) { this->_targetParamTypeToken.push_back(0); } std::vector filterValues; @@ -576,7 +574,7 @@ namespace iast context.instruction = inserted; } } - else // Replace + else //Replace { if (_aspect->_paramShift.size() > 0) { @@ -587,8 +585,7 @@ namespace iast continue; } - auto instructionInfo = - processor->StackAnalysis()->GetInstruction(instructionToProcess.instruction); + auto instructionInfo = processor->StackAnalysis()->GetInstruction(instructionToProcess.instruction); auto methodSig = instructionInfo->GetArgumentSignature(); int paramCount = methodSig->GetEffectiveParamCount(); for (auto iInfo : processor->StackAnalysis()->LocateCallParamInstructions( @@ -596,8 +593,7 @@ namespace iast paramCount - _aspect->_paramShift[x] - 1)) // Locate param load instruction { auto paramType = _targetParamTypeToken[x]; - processor->InsertAfter(iInfo->_instruction, - processor->NewILInstr(CEE_BOX, paramType, true)); + processor->InsertAfter(iInfo->_instruction, processor->NewILInstr(CEE_BOX, paramType, true)); // Figure out if param is byref if (iInfo->IsArgument()) @@ -606,8 +602,7 @@ namespace iast auto param = sig->_params[iInfo->_instruction->m_Arg32]; if (param->IsByRef()) { - processor->InsertAfter(iInfo->_instruction, - processor->NewILInstr(CEE_LDOBJ, paramType, true)); + processor->InsertAfter(iInfo->_instruction, processor->NewILInstr(CEE_LDOBJ, paramType, true)); } } diff --git a/tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.h b/tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.h index 15d5f0e07689..372ebd523ff3 100644 --- a/tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.h +++ b/tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.h @@ -41,7 +41,7 @@ namespace iast protected: WSTRING _line = EmptyWStr; bool _isValid = false; - + mdTypeDef _aspectTypeDef = 0; public: Dataflow* _dataflow; WSTRING _aspectsAssembly; @@ -122,7 +122,7 @@ namespace iast class DataflowAspectReference { public: - DataflowAspectReference(ModuleAspects* moduleAspects, DataflowAspect* aspect, mdMemberRef targetMethod, mdTypeRef targetType, bool isRefStruct, const std::vector& paramType); + DataflowAspectReference(ModuleAspects* moduleAspects, DataflowAspect* aspect, mdMemberRef targetMethod, mdTypeRef targetType, const std::vector& paramType); public: DataflowAspect* _aspect = nullptr; @@ -130,7 +130,6 @@ namespace iast ModuleInfo* _module = nullptr; mdMemberRef _targetMethodRef = 0; mdTypeRef _targetTypeRef = 0; - bool _isRefStruct = false; std::vector _targetParamTypeToken; std::vector _filters; From b9ea3d2d6739631b0e1b88f8759b801d9800f36a Mon Sep 17 00:00:00 2001 From: Flavien Darche Date: Sun, 24 Nov 2024 20:31:53 +0100 Subject: [PATCH 10/16] fix snapshot --- ...liInterpolatedString.AspNetCore5.IastEnabled.verified.txt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tracer/test/snapshots/Iast.SqliInterpolatedString.AspNetCore5.IastEnabled.verified.txt b/tracer/test/snapshots/Iast.SqliInterpolatedString.AspNetCore5.IastEnabled.verified.txt index 2dcde2ce3397..ca5e201e0aeb 100644 --- a/tracer/test/snapshots/Iast.SqliInterpolatedString.AspNetCore5.IastEnabled.verified.txt +++ b/tracer/test/snapshots/Iast.SqliInterpolatedString.AspNetCore5.IastEnabled.verified.txt @@ -30,8 +30,7 @@ "location": { "spanId": XXX, "path": "Samples.Security.AspNetCore5.Controllers.IastController", - "method": "InterpolatedSqlString", - "line": XXX + "method": "InterpolatedSqlString" }, "evidence": { "valueParts": [ @@ -43,7 +42,7 @@ "source": 0 }, { - "value": "'3','32,38','Vins et alcools Chevalier','John'," + "value": "'3','32.38','Vins et alcools Chevalier','John'," }, { "value": "", From 30dfe3ea54544e9cdf7d86a8520ad53d97f8de7e Mon Sep 17 00:00:00 2001 From: Flavien Darche Date: Mon, 25 Nov 2024 11:18:52 +0100 Subject: [PATCH 11/16] Update to set the tainted string as the result length + update marks of range --- .../DefaultInterpolatedStringHandlerModuleImpl.cs | 8 ++++---- ...terpolatedString.AspNetCore5.IastEnabled.verified.txt | 9 +++------ .../Controllers/IastController.cs | 2 +- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/tracer/src/Datadog.Trace/Iast/Propagation/DefaultInterpolatedStringHandlerModuleImpl.cs b/tracer/src/Datadog.Trace/Iast/Propagation/DefaultInterpolatedStringHandlerModuleImpl.cs index 64fe026b8910..b24a09c8d7b9 100644 --- a/tracer/src/Datadog.Trace/Iast/Propagation/DefaultInterpolatedStringHandlerModuleImpl.cs +++ b/tracer/src/Datadog.Trace/Iast/Propagation/DefaultInterpolatedStringHandlerModuleImpl.cs @@ -48,8 +48,7 @@ public static unsafe void FullTaintIfAnyTainted(IntPtr target, string? input) return; } - // The start and length here are not correct, but we don't currently have a way to get the correct values - var rangesResult = new[] { new Range(0, 0, tainted!.Ranges[0].Source) }; + var rangesResult = new[] { new Range(0, 0, tainted!.Ranges[0].Source, tainted.Ranges[0].SecureMarks) }; if (!targetIsTainted) { taintedObjects.Taint(target, rangesResult); @@ -65,7 +64,7 @@ public static unsafe void FullTaintIfAnyTainted(IntPtr target, string? input) } } - public static object? PropagateTaint(object? input, object? result) + public static object? PropagateTaint(object? input, string? result) { try { @@ -90,7 +89,8 @@ public static unsafe void FullTaintIfAnyTainted(IntPtr target, string? input) return result; } - taintedObjects.Taint(result, taintedSelf.Ranges); + var range = new Range(0, result.Length, taintedSelf.Ranges[0].Source, taintedSelf.Ranges[0].SecureMarks); + taintedObjects.Taint(result, [range]); } catch (Exception err) { diff --git a/tracer/test/snapshots/Iast.SqliInterpolatedString.AspNetCore5.IastEnabled.verified.txt b/tracer/test/snapshots/Iast.SqliInterpolatedString.AspNetCore5.IastEnabled.verified.txt index ca5e201e0aeb..1df988095c8c 100644 --- a/tracer/test/snapshots/Iast.SqliInterpolatedString.AspNetCore5.IastEnabled.verified.txt +++ b/tracer/test/snapshots/Iast.SqliInterpolatedString.AspNetCore5.IastEnabled.verified.txt @@ -38,18 +38,15 @@ "value": "INSERT INTO Orders (CustomerId, EmployeeId, OrderDate, RequiredDate, ShipVia, Freight, ShipName, ShipAddress, ShipCity, ShipPostalCode, ShipCountry) VALUES ('VINET','5','2021-01-01','2021-01-01'," }, { - "value": "", + "value": "'3','32.38','Vins et alcools Chevalier','John',", "source": 0 }, { - "value": "'3','32.38','Vins et alcools Chevalier','John'," - }, - { - "value": "", + "value": "'Reims','51100','France')", "source": 0 }, { - "value": "'Reims','51100','France');\nSELECT OrderID FROM Orders ORDER BY OrderID DESC LIMIT 1;" + "value": ";\nSELECT OrderID FROM Orders ORDER BY OrderID DESC LIMIT 1;" } ] } diff --git a/tracer/test/test-applications/security/Samples.Security.AspNetCore5/Controllers/IastController.cs b/tracer/test/test-applications/security/Samples.Security.AspNetCore5/Controllers/IastController.cs index 9616d8cd66e0..6d945278a25f 100644 --- a/tracer/test/test-applications/security/Samples.Security.AspNetCore5/Controllers/IastController.cs +++ b/tracer/test/test-applications/security/Samples.Security.AspNetCore5/Controllers/IastController.cs @@ -1099,7 +1099,7 @@ public IActionResult InterpolatedSqlString(string name) { new SqliteCommand(sql, DbConnectionSystemDataMicrosoftData).ExecuteScalar(); } - catch (Exception ex) + catch (Exception) { // ignored } From e777c4937dc98c8d488092a8e4a599da4320775a Mon Sep 17 00:00:00 2001 From: Flavien Darche Date: Tue, 26 Nov 2024 18:01:41 +0100 Subject: [PATCH 12/16] Applied comment --- .../DefaultInterpolatedStringHandlerAspect.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs b/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs index c7ce264d28dd..832a0724a758 100644 --- a/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs +++ b/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs @@ -5,6 +5,8 @@ #if NET6_0_OR_GREATER +#nullable enable + using System; using System.Reflection.Emit; using System.Runtime.CompilerServices; @@ -13,8 +15,6 @@ using InlineIL; using static InlineIL.IL.Emit; -#nullable enable - namespace Datadog.Trace.Iast.Aspects.System.Runtime; /// DefaultInterpolatedString class aspect @@ -38,7 +38,7 @@ public static void AppendFormatted1(ref DefaultInterpolatedStringHandler target, } catch (Exception ex) { - IastModule.Log.Error(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted1)}"); + IastModule.Log.Warning(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted1)}"); } } @@ -61,7 +61,7 @@ public static void AppendFormatted2(ref DefaultInterpolatedStringHandler target, } catch (Exception ex) { - IastModule.Log.Error(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted2)}"); + IastModule.Log.Warning(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted2)}"); } } @@ -82,7 +82,7 @@ public static void AppendFormatted3(ref DefaultInterpolatedStringHandler target, } catch (Exception ex) { - IastModule.Log.Error(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted3)}"); + IastModule.Log.Warning(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted3)}"); } } @@ -101,7 +101,7 @@ public static void AppendFormatted4(ref DefaultInterpolatedStringHandler target, } catch (Exception ex) { - IastModule.Log.Error(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted4)}"); + IastModule.Log.Warning(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted5)}"); } } @@ -120,7 +120,7 @@ public static void AppendLiteral(ref DefaultInterpolatedStringHandler target, st } catch (Exception ex) { - IastModule.Log.Error(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendLiteral)}"); + IastModule.Log.Warning(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendLiteral)}"); } } @@ -139,7 +139,7 @@ public static string ToStringAndClear(ref DefaultInterpolatedStringHandler targe } catch (Exception ex) { - IastModule.Log.Error(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(ToStringAndClear)}"); + IastModule.Log.Warning(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(ToStringAndClear)}"); } return result; From c7ffef0beab7de91ceca87124e9c781d7da696dc Mon Sep 17 00:00:00 2001 From: Flavien Darche Date: Tue, 26 Nov 2024 18:16:32 +0100 Subject: [PATCH 13/16] not working aspect --- .../DefaultInterpolatedStringHandlerAspect.cs | 16 ++++++++++------ .../Generated/generated_callsites.g.h | 2 +- .../DefaultInterpolatedStringHandlerTests.cs | 17 +++++++++++++++++ 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs b/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs index 832a0724a758..bcd8fdd1b9fe 100644 --- a/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs +++ b/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs @@ -87,21 +87,25 @@ public static void AppendFormatted3(ref DefaultInterpolatedStringHandler target, } /// - /// System.Runtime DefaultInterpolatedStringHandler.AppendFormatted(String, Int32, String) aspect + /// System.Runtime DefaultInterpolatedStringHandler.AppendFormatted(!!0) aspect /// + /// The first generic type parameter. /// the ref DefaultInterpolatedStringHandler /// the string value - [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0)")] - public static void AppendFormatted4(ref DefaultInterpolatedStringHandler target, string value) + [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0)")] + public static void AppendFormatted4(ref DefaultInterpolatedStringHandler target, T value) { - target.AppendFormatted(value); + target.AppendFormatted(value); try { - DefaultInterpolatedStringHandlerModuleImpl.Append(ToPointer(ref target), value); + if (value is string str) + { + DefaultInterpolatedStringHandlerModuleImpl.Append(ToPointer(ref target), str); + } } catch (Exception ex) { - IastModule.Log.Warning(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted5)}"); + IastModule.Log.Warning(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted4)}"); } } diff --git a/tracer/src/Datadog.Tracer.Native/Generated/generated_callsites.g.h b/tracer/src/Datadog.Tracer.Native/Generated/generated_callsites.g.h index 473f528d6d37..848daf75119b 100644 --- a/tracer/src/Datadog.Tracer.Native/Generated/generated_callsites.g.h +++ b/tracer/src/Datadog.Tracer.Native/Generated/generated_callsites.g.h @@ -625,7 +625,7 @@ std::vector g_callSites= (WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0,System.Int32)\",\"\",[0],[False],[None],Default,[])] AppendFormatted2(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.Object,System.Int32) 128"), (WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.String)\",\"\",[0],[False],[None],Default,[])] AppendFormatted1(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.String) 128"), (WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.String,System.Int32,System.String)\",\"\",[0],[False],[None],Default,[])] AppendFormatted3(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.String,System.Int32,System.String) 128"), -(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0)\",\"\",[0],[False],[None],Default,[])] AppendFormatted4(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.String) 128"), +(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0)\",\"\",[0],[False],[None],Default,[])] AppendFormatted4(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,T) 128"), (WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendLiteral(System.String)\",\"\",[0],[False],[None],Default,[])] AppendLiteral(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.String) 128"), (WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::ToStringAndClear()\",\"\",[0],[False],[None],Default,[])] ToStringAndClear(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler) 128"), (WCHAR*)WStr("[AspectClass(\"System.Text.Json\",[None],Source,[])] Datadog.Trace.Iast.Aspects.System.Text.Json.JsonDocumentAspects 4"), diff --git a/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringHandlerTests.cs b/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringHandlerTests.cs index b1506713ad7d..cf0b8761f409 100644 --- a/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringHandlerTests.cs +++ b/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringHandlerTests.cs @@ -70,6 +70,15 @@ public void GivenAnExplicitInterpolatedString_WhenAddingUntaintedValueAppendForm AssertNotTainted(test.ToStringAndClear()); } + [Fact] + public void GivenAnExplicitInterpolatedString_WhenAddingTaintedValueAppendFormattedTObject_GetString_Vulnerable() + { + var test = new DefaultInterpolatedStringHandler(); + test.AppendFormatted((object)TaintedValue); + + AssertTainted(test.ToStringAndClear()); + } + [Fact] public void GivenAnExplicitInterpolatedString_WhenAddingTaintedValueAppendLiteral_GetString_Vulnerable() { @@ -116,6 +125,14 @@ public void GivenAnImplicitInterpolatedString_WhenAddingUntaintedValue_GetString AssertNotTainted(str); } + [Fact] + public void GivenAnImplicitInterpolatedString_WhenAddingTaintedValueAsObject_GetString_Vulnerable() + { + var number = 5; + var str = $"Hello {(object)TaintedValue} {number}"; + AssertTainted(str); + } + [Fact] public void GivenAnImplicitInterpolatedString_WhenAddingMultipleValuesWithTaintedValues_GetString_Vulnerable() { From 2c974d20f60a64e35b9c2742b37522a3b09c03ba Mon Sep 17 00:00:00 2001 From: Daniel Romano <108014683+daniel-romano-DD@users.noreply.github.com> Date: Wed, 27 Nov 2024 01:01:21 +0100 Subject: [PATCH 14/16] Fixed broken generic aspects support --- .../_build/CodeGenerators/CallSitesGenerator.cs | 14 +++++++++++++- .../DefaultInterpolatedStringHandlerAspect.cs | 5 +++-- .../Generated/generated_callsites.g.h | 4 ++-- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/tracer/build/_build/CodeGenerators/CallSitesGenerator.cs b/tracer/build/_build/CodeGenerators/CallSitesGenerator.cs index 428b389584a9..17619ee439b3 100644 --- a/tracer/build/_build/CodeGenerators/CallSitesGenerator.cs +++ b/tracer/build/_build/CodeGenerators/CallSitesGenerator.cs @@ -75,7 +75,9 @@ internal static void RetrieveCallSites(Dictionary aspectCla static string GetMethodName(MethodDefinition method) { - var fullName = method.FullName; + var parameters = string.Join(",", method.Parameters.Select(GetParameter)); + var fullName = $"{method.FullName.Substring(0, method.FullName.IndexOf('('))}({parameters})"; + var methodNameStart = fullName.IndexOf("::"); if (methodNameStart < 0) { @@ -83,6 +85,16 @@ static string GetMethodName(MethodDefinition method) } return fullName.Substring(methodNameStart + 2).Replace("", "").Replace("&", ""); + + static string GetParameter(ParameterDefinition parameter) + { + var paramType = parameter.ParameterType.FullName; + return paramType switch + { + "T" => "!!0", + _ => paramType.Replace("", ""), + }; + } } static bool IsAspectClass(Mono.Cecil.CustomAttribute attribute) diff --git a/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs b/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs index bcd8fdd1b9fe..84ea35b39be3 100644 --- a/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs +++ b/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs @@ -45,11 +45,12 @@ public static void AppendFormatted1(ref DefaultInterpolatedStringHandler target, /// /// System.Runtime DefaultInterpolatedStringHandler.AppendFormatted(T, String) aspect /// + /// The first generic type parameter. /// the ref DefaultInterpolatedStringHandler /// the string value /// the alignment value [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0,System.Int32)")] - public static void AppendFormatted2(ref DefaultInterpolatedStringHandler target, object value, int alignment) + public static void AppendFormatted2(ref DefaultInterpolatedStringHandler target, T value, int alignment) { target.AppendFormatted(value, alignment); try @@ -92,7 +93,7 @@ public static void AppendFormatted3(ref DefaultInterpolatedStringHandler target, /// The first generic type parameter. /// the ref DefaultInterpolatedStringHandler /// the string value - [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0)")] + [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0)")] public static void AppendFormatted4(ref DefaultInterpolatedStringHandler target, T value) { target.AppendFormatted(value); diff --git a/tracer/src/Datadog.Tracer.Native/Generated/generated_callsites.g.h b/tracer/src/Datadog.Tracer.Native/Generated/generated_callsites.g.h index 848daf75119b..bda7cf5f6db5 100644 --- a/tracer/src/Datadog.Tracer.Native/Generated/generated_callsites.g.h +++ b/tracer/src/Datadog.Tracer.Native/Generated/generated_callsites.g.h @@ -622,10 +622,10 @@ std::vector g_callSites= (WCHAR*)WStr(" [AspectMethodReplace(\"System.Net.WebUtility::HtmlEncode(System.String)\",\"\",[0],[False],[None],Default,[])] XssEscape(System.String) 165"), (WCHAR*)WStr(" [AspectMethodReplace(\"System.Net.WebUtility::UrlEncode(System.String)\",\"\",[0],[False],[None],Default,[])] SsrfEscape(System.String) 165"), (WCHAR*)WStr("[AspectClass(\"System.Runtime\",[None],Propagation,[])] Datadog.Trace.Iast.Aspects.System.Runtime.DefaultInterpolatedStringHandlerAspect 4"), -(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0,System.Int32)\",\"\",[0],[False],[None],Default,[])] AppendFormatted2(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.Object,System.Int32) 128"), +(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0)\",\"\",[0],[False],[None],Default,[])] AppendFormatted4(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,!!0) 128"), +(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0,System.Int32)\",\"\",[0],[False],[None],Default,[])] AppendFormatted2(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,!!0,System.Int32) 128"), (WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.String)\",\"\",[0],[False],[None],Default,[])] AppendFormatted1(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.String) 128"), (WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.String,System.Int32,System.String)\",\"\",[0],[False],[None],Default,[])] AppendFormatted3(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.String,System.Int32,System.String) 128"), -(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0)\",\"\",[0],[False],[None],Default,[])] AppendFormatted4(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,T) 128"), (WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendLiteral(System.String)\",\"\",[0],[False],[None],Default,[])] AppendLiteral(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.String) 128"), (WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::ToStringAndClear()\",\"\",[0],[False],[None],Default,[])] ToStringAndClear(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler) 128"), (WCHAR*)WStr("[AspectClass(\"System.Text.Json\",[None],Source,[])] Datadog.Trace.Iast.Aspects.System.Text.Json.JsonDocumentAspects 4"), From 15c9439e437d440c804d23da9c415f8b32e4b285 Mon Sep 17 00:00:00 2001 From: Flavien Darche Date: Wed, 27 Nov 2024 11:36:38 +0100 Subject: [PATCH 15/16] Add generic aspects + handle tests --- .../DefaultInterpolatedStringHandlerAspect.cs | 101 +++++++++++-- .../Generated/generated_callsites.g.h | 7 +- .../DefaultInterpolatedStringHandlerTests.cs | 142 ++++++++++++++---- 3 files changed, 203 insertions(+), 47 deletions(-) diff --git a/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs b/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs index 84ea35b39be3..9f38177b8c49 100644 --- a/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs +++ b/tracer/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs @@ -42,6 +42,74 @@ public static void AppendFormatted1(ref DefaultInterpolatedStringHandler target, } } + /// + /// System.Runtime DefaultInterpolatedStringHandler.AppendFormatted(String, Int32, String) aspect + /// + /// the ref DefaultInterpolatedStringHandler + /// the string value + /// the alignment value + /// the format value + [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.String,System.Int32,System.String)")] + public static void AppendFormatted2(ref DefaultInterpolatedStringHandler target, string? value, int alignment, string? format) + { + target.AppendFormatted(value, alignment, format); + try + { + DefaultInterpolatedStringHandlerModuleImpl.Append(ToPointer(ref target), value); + } + catch (Exception ex) + { + IastModule.Log.Warning(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted2)}"); + } + } + + /// + /// System.Runtime DefaultInterpolatedStringHandler.AppendFormatted(Object, Int32, String) aspect + /// + /// the ref DefaultInterpolatedStringHandler + /// the object value + /// the alignment value + /// the format value + [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.Object,System.Int32,System.String)")] + public static void AppendFormatted3(ref DefaultInterpolatedStringHandler target, object? value, int alignment, string? format) + { + target.AppendFormatted(value, alignment, format); + try + { + if (value is string str) + { + DefaultInterpolatedStringHandlerModuleImpl.Append(ToPointer(ref target), str); + } + } + catch (Exception ex) + { + IastModule.Log.Warning(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted3)}"); + } + } + + /// + /// System.Runtime DefaultInterpolatedStringHandler.AppendFormatted(T) aspect + /// + /// The first generic type parameter. + /// the ref DefaultInterpolatedStringHandler + /// the string value + [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0)")] + public static void AppendFormatted4(ref DefaultInterpolatedStringHandler target, T value) + { + target.AppendFormatted(value); + try + { + if (value is string str) + { + DefaultInterpolatedStringHandlerModuleImpl.Append(ToPointer(ref target), str); + } + } + catch (Exception ex) + { + IastModule.Log.Warning(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted4)}"); + } + } + /// /// System.Runtime DefaultInterpolatedStringHandler.AppendFormatted(T, String) aspect /// @@ -50,7 +118,7 @@ public static void AppendFormatted1(ref DefaultInterpolatedStringHandler target, /// the string value /// the alignment value [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0,System.Int32)")] - public static void AppendFormatted2(ref DefaultInterpolatedStringHandler target, T value, int alignment) + public static void AppendFormatted5(ref DefaultInterpolatedStringHandler target, T value, int alignment) { target.AppendFormatted(value, alignment); try @@ -62,41 +130,46 @@ public static void AppendFormatted2(ref DefaultInterpolatedStringHandler targ } catch (Exception ex) { - IastModule.Log.Warning(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted2)}"); + IastModule.Log.Warning(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted5)}"); } } /// - /// System.Runtime DefaultInterpolatedStringHandler.AppendFormatted(String, Int32, String) aspect + /// System.Runtime DefaultInterpolatedStringHandler.AppendFormatted(T, String) aspect /// + /// The first generic type parameter. /// the ref DefaultInterpolatedStringHandler /// the string value - /// the alignment value /// the format value - [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.String,System.Int32,System.String)")] - public static void AppendFormatted3(ref DefaultInterpolatedStringHandler target, string? value, int alignment, string? format) + [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0,System.String)")] + public static void AppendFormatted6(ref DefaultInterpolatedStringHandler target, T value, string? format) { - target.AppendFormatted(value, alignment, format); + target.AppendFormatted(value, format); try { - DefaultInterpolatedStringHandlerModuleImpl.Append(ToPointer(ref target), value); + if (value is string str) + { + DefaultInterpolatedStringHandlerModuleImpl.Append(ToPointer(ref target), str); + } } catch (Exception ex) { - IastModule.Log.Warning(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted3)}"); + IastModule.Log.Warning(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted6)}"); } } /// - /// System.Runtime DefaultInterpolatedStringHandler.AppendFormatted(!!0) aspect + /// System.Runtime DefaultInterpolatedStringHandler.AppendFormatted(T, String) aspect /// /// The first generic type parameter. /// the ref DefaultInterpolatedStringHandler /// the string value - [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0)")] - public static void AppendFormatted4(ref DefaultInterpolatedStringHandler target, T value) + /// the alignment value + /// the format value + [AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0,System.Int32,System.String)")] + public static void AppendFormatted7(ref DefaultInterpolatedStringHandler target, T value, int alignment, string? format) { - target.AppendFormatted(value); + target.AppendFormatted(value, alignment, format); try { if (value is string str) @@ -106,7 +179,7 @@ public static void AppendFormatted4(ref DefaultInterpolatedStringHandler targ } catch (Exception ex) { - IastModule.Log.Warning(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted4)}"); + IastModule.Log.Warning(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted7)}"); } } diff --git a/tracer/src/Datadog.Tracer.Native/Generated/generated_callsites.g.h b/tracer/src/Datadog.Tracer.Native/Generated/generated_callsites.g.h index bda7cf5f6db5..1b82dda66566 100644 --- a/tracer/src/Datadog.Tracer.Native/Generated/generated_callsites.g.h +++ b/tracer/src/Datadog.Tracer.Native/Generated/generated_callsites.g.h @@ -623,9 +623,12 @@ std::vector g_callSites= (WCHAR*)WStr(" [AspectMethodReplace(\"System.Net.WebUtility::UrlEncode(System.String)\",\"\",[0],[False],[None],Default,[])] SsrfEscape(System.String) 165"), (WCHAR*)WStr("[AspectClass(\"System.Runtime\",[None],Propagation,[])] Datadog.Trace.Iast.Aspects.System.Runtime.DefaultInterpolatedStringHandlerAspect 4"), (WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0)\",\"\",[0],[False],[None],Default,[])] AppendFormatted4(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,!!0) 128"), -(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0,System.Int32)\",\"\",[0],[False],[None],Default,[])] AppendFormatted2(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,!!0,System.Int32) 128"), +(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0,System.Int32)\",\"\",[0],[False],[None],Default,[])] AppendFormatted5(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,!!0,System.Int32) 128"), +(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0,System.Int32,System.String)\",\"\",[0],[False],[None],Default,[])] AppendFormatted7(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,!!0,System.Int32,System.String) 128"), +(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0,System.String)\",\"\",[0],[False],[None],Default,[])] AppendFormatted6(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,!!0,System.String) 128"), +(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.Object,System.Int32,System.String)\",\"\",[0],[False],[None],Default,[])] AppendFormatted3(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.Object,System.Int32,System.String) 128"), (WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.String)\",\"\",[0],[False],[None],Default,[])] AppendFormatted1(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.String) 128"), -(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.String,System.Int32,System.String)\",\"\",[0],[False],[None],Default,[])] AppendFormatted3(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.String,System.Int32,System.String) 128"), +(WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.String,System.Int32,System.String)\",\"\",[0],[False],[None],Default,[])] AppendFormatted2(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.String,System.Int32,System.String) 128"), (WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendLiteral(System.String)\",\"\",[0],[False],[None],Default,[])] AppendLiteral(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler,System.String) 128"), (WCHAR*)WStr(" [AspectMethodReplace(\"System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::ToStringAndClear()\",\"\",[0],[False],[None],Default,[])] ToStringAndClear(System.Runtime.CompilerServices.DefaultInterpolatedStringHandler) 128"), (WCHAR*)WStr("[AspectClass(\"System.Text.Json\",[None],Source,[])] Datadog.Trace.Iast.Aspects.System.Text.Json.JsonDocumentAspects 4"), diff --git a/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringHandlerTests.cs b/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringHandlerTests.cs index cf0b8761f409..652aa17848bb 100644 --- a/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringHandlerTests.cs +++ b/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringHandlerTests.cs @@ -3,6 +3,7 @@ using System; using Xunit; using System.Runtime.CompilerServices; +using FluentAssertions; namespace Samples.InstrumentedTests.Iast.Vulnerabilities.StringPropagation; @@ -15,86 +16,159 @@ public DefaultInterpolatedStringHandlerTests() { AddTainted(TaintedValue); } - + [Fact] public void GivenAnExplicitInterpolatedString_WhenAddingTaintedValueAppendFormatted1_GetString_Vulnerable() { var test = new DefaultInterpolatedStringHandler(); test.AppendFormatted(TaintedValue); - AssertTainted(test.ToStringAndClear()); + var str = test.ToStringAndClear(); + str.Should().Be(TaintedValue); + AssertTainted(str); } - + [Fact] public void GivenAnExplicitInterpolatedString_WhenAddingUntaintedValueAppendFormatted1_GetString_NonVulnerable() { var test = new DefaultInterpolatedStringHandler(); test.AppendFormatted(UntaintedValue); - AssertNotTainted(test.ToStringAndClear()); + var str = test.ToStringAndClear(); + str.Should().Be(UntaintedValue); + AssertNotTainted(str); } - + [Fact] public void GivenAnExplicitInterpolatedString_WhenAddingTaintedValueAppendFormatted2_GetString_Vulnerable() { var test = new DefaultInterpolatedStringHandler(); - test.AppendFormatted(TaintedValue, 0); + test.AppendFormatted(TaintedValue, 0, string.Empty); - AssertTainted(test.ToStringAndClear()); + var str = test.ToStringAndClear(); + str.Should().Be(TaintedValue); + AssertTainted(str); } - + [Fact] public void GivenAnExplicitInterpolatedString_WhenAddingUntaintedValueAppendFormatted2_GetString_NonVulnerable() { var test = new DefaultInterpolatedStringHandler(); - test.AppendFormatted(UntaintedValue, 0); + test.AppendFormatted(UntaintedValue, 0, string.Empty); - AssertNotTainted(test.ToStringAndClear()); + var str = test.ToStringAndClear(); + str.Should().Be(UntaintedValue); + AssertNotTainted(str); } - + [Fact] public void GivenAnExplicitInterpolatedString_WhenAddingTaintedValueAppendFormatted3_GetString_Vulnerable() { var test = new DefaultInterpolatedStringHandler(); - test.AppendFormatted(TaintedValue, 0, null); + test.AppendFormatted((object)TaintedValue, 0, string.Empty); - AssertTainted(test.ToStringAndClear()); + var str = test.ToStringAndClear(); + str.Should().Be(TaintedValue); + AssertTainted(str); } - + [Fact] public void GivenAnExplicitInterpolatedString_WhenAddingUntaintedValueAppendFormatted3_GetString_NonVulnerable() { var test = new DefaultInterpolatedStringHandler(); - test.AppendFormatted(UntaintedValue, 0, null); + test.AppendFormatted((object)UntaintedValue, 0, string.Empty); - AssertNotTainted(test.ToStringAndClear()); + var str = test.ToStringAndClear(); + str.Should().Be(UntaintedValue); + AssertNotTainted(str); } - + [Fact] - public void GivenAnExplicitInterpolatedString_WhenAddingTaintedValueAppendFormattedTObject_GetString_Vulnerable() + public void GivenAnExplicitInterpolatedString_WhenAddingTaintedValueAppendFormatted4_GetString_Vulnerable() { var test = new DefaultInterpolatedStringHandler(); test.AppendFormatted((object)TaintedValue); - AssertTainted(test.ToStringAndClear()); + var str = test.ToStringAndClear(); + str.Should().Be(TaintedValue); + AssertTainted(str); } - + [Fact] - public void GivenAnExplicitInterpolatedString_WhenAddingTaintedValueAppendLiteral_GetString_Vulnerable() + public void GivenAnExplicitInterpolatedString_WhenAddingUntaintedValueAppendFormatted4_GetString_NonVulnerable() { var test = new DefaultInterpolatedStringHandler(); - test.AppendLiteral(TaintedValue); + test.AppendFormatted((object)UntaintedValue); - AssertTainted(test.ToStringAndClear()); + var str = test.ToStringAndClear(); + str.Should().Be(UntaintedValue); + AssertNotTainted(str); + } + + [Fact] + public void GivenAnExplicitInterpolatedString_WhenAddingTaintedValueAppendFormatted5_GetString_Vulnerable() + { + var test = new DefaultInterpolatedStringHandler(); + test.AppendFormatted((object)TaintedValue, 0); + + var str = test.ToStringAndClear(); + str.Should().Be(TaintedValue); + AssertTainted(str); + } + + [Fact] + public void GivenAnExplicitInterpolatedString_WhenAddingUntaintedValueAppendFormatted5_GetString_NonVulnerable() + { + var test = new DefaultInterpolatedStringHandler(); + test.AppendFormatted((object)UntaintedValue, 0); + + var str = test.ToStringAndClear(); + str.Should().Be(UntaintedValue); + AssertNotTainted(str); + } + + [Fact] + public void GivenAnExplicitInterpolatedString_WhenAddingTaintedValueAppendFormatted6_GetString_Vulnerable() + { + var test = new DefaultInterpolatedStringHandler(); + test.AppendFormatted((object)TaintedValue, string.Empty); + + var str = test.ToStringAndClear(); + str.Should().Be(TaintedValue); + AssertTainted(str); + } + + [Fact] + public void GivenAnExplicitInterpolatedString_WhenAddingUntaintedValueAppendFormatted6_GetString_NonVulnerable() + { + var test = new DefaultInterpolatedStringHandler(); + test.AppendFormatted((object)UntaintedValue, string.Empty); + + var str = test.ToStringAndClear(); + str.Should().Be(UntaintedValue); + AssertNotTainted(str); } [Fact] - public void GivenAnExplicitInterpolatedString_WhenAddingUntaintedValueAppendLiteral_GetString_NonVulnerable() + public void GivenAnExplicitInterpolatedString_WhenAddingTaintedValueAppendFormatted7_GetString_Vulnerable() { var test = new DefaultInterpolatedStringHandler(); - test.AppendFormatted(UntaintedValue); + test.AppendFormatted((object)TaintedValue, 0, string.Empty); + + var str = test.ToStringAndClear(); + str.Should().Be(TaintedValue); + AssertTainted(str); + } + + [Fact] + public void GivenAnExplicitInterpolatedString_WhenAddingUntaintedValueAppendFormatted7_GetString_NonVulnerable() + { + var test = new DefaultInterpolatedStringHandler(); + test.AppendFormatted((object)UntaintedValue, 0, string.Empty); - AssertNotTainted(test.ToStringAndClear()); + var str = test.ToStringAndClear(); + str.Should().Be(UntaintedValue); + AssertNotTainted(str); } [Fact] @@ -108,12 +182,13 @@ public void GivenAnExplicitInterpolatedString_WhenAddingTaintedValueMultipleValu AssertTainted(test.ToStringAndClear()); } - + [Fact] public void GivenAnImplicitInterpolatedString_WhenAddingTaintedValue_GetString_Vulnerable() { var number = 5; var str = $"Hello {TaintedValue} {number}"; + str.Should().Be("Hello " + TaintedValue + " " + number); AssertTainted(str); } @@ -122,17 +197,19 @@ public void GivenAnImplicitInterpolatedString_WhenAddingUntaintedValue_GetString { var number = 5; var str = $"Hello {UntaintedValue} {number}"; + str.Should().Be("Hello " + UntaintedValue + " " + number); AssertNotTainted(str); } - + [Fact] public void GivenAnImplicitInterpolatedString_WhenAddingTaintedValueAsObject_GetString_Vulnerable() { var number = 5; var str = $"Hello {(object)TaintedValue} {number}"; + str.Should().Be("Hello " + TaintedValue + " " + number); AssertTainted(str); } - + [Fact] public void GivenAnImplicitInterpolatedString_WhenAddingMultipleValuesWithTaintedValues_GetString_Vulnerable() { @@ -143,7 +220,7 @@ public void GivenAnImplicitInterpolatedString_WhenAddingMultipleValuesWithTainte OrderDate = new DateTime(2021, 1, 1), RequiredDate = new DateTime(2021, 1, 1), ShipVia = 3, - Freight = 32.38M, + Freight = 32, ShipName = "Vins et alcools Chevalier", ShipAddress = TaintedValue, ShipCity = "Reims", @@ -159,9 +236,10 @@ public void GivenAnImplicitInterpolatedString_WhenAddingMultipleValuesWithTainte $"'{order.ShipVia}','{order.Freight}','{order.ShipName}','{order.ShipAddress}'," + $"'{order.ShipCity}','{order.ShipPostalCode}','{order.ShipCountry}')"; + sql.Should().Be("INSERT INTO Orders (CustomerId, EmployeeId, OrderDate, RequiredDate, ShipVia, Freight, ShipName, ShipAddress, ShipCity, ShipPostalCode, ShipCountry) VALUES ('VINET','5','2021-01-01','2021-01-01','3','32','Vins et alcools Chevalier','tainted','Reims','51100','France')"); AssertTainted(sql); } - + [Fact] public void GivenImplicitInterpolatedString_WhenAddingTaintedValuesNested_GetString_Vulnerable() { @@ -177,6 +255,7 @@ public void GivenImplicitInterpolatedString_WhenAddingTaintedValuesNested_GetStr var nestedString = $"Hello {$"{TaintedValue + "Hello"} - {complexString}"}"; var finalString = $"Final {nestedString} and char {charValue} with additional {UntaintedValue} and number {number} and date {date:HH:mm:ss}"; + finalString.Should().Be("Final Hello Hello - Complex Nested2 Nested1 tainted and 42 with date 2024-11-22 and decimal 123.46 and boolean True and char A with additional untainted and number 42 and date 15:30:00"); AssertTainted(finalString); } @@ -187,6 +266,7 @@ public void GivenImplicitInterpolatedString_WhenAddingTaintedValuesComplex_GetSt Hello "{TaintedValue}" and "{UntaintedValue}". . """; + interpolatedString.Should().Be("Hello \"tainted\" and \"untainted\".\n."); AssertTainted(interpolatedString); } } From bfdfd644710b0d905ba311fb44fd7f8eba2e6c69 Mon Sep 17 00:00:00 2001 From: Flavien Darche Date: Wed, 27 Nov 2024 12:57:09 +0100 Subject: [PATCH 16/16] Add more tests --- .../IAST/IastInstrumentationUnitTests.cs | 17 +++++++++++++++-- .../DefaultInterpolatedStringHandlerTests.cs | 11 +++++------ 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/tracer/test/Datadog.Trace.Security.IntegrationTests/IAST/IastInstrumentationUnitTests.cs b/tracer/test/Datadog.Trace.Security.IntegrationTests/IAST/IastInstrumentationUnitTests.cs index 671bc9b76d07..bd704e912ae6 100644 --- a/tracer/test/Datadog.Trace.Security.IntegrationTests/IAST/IastInstrumentationUnitTests.cs +++ b/tracer/test/Datadog.Trace.Security.IntegrationTests/IAST/IastInstrumentationUnitTests.cs @@ -15,6 +15,7 @@ using System.Net.Http; using System.Net.Mail; using System.Reflection; +using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Security.Cryptography; using System.Text; @@ -150,6 +151,9 @@ public List GetAspects() [InlineData(typeof(SmtpClient), "Send", new[] { "Void Send(System.String, System.String, System.String, System.String)" }, true)] [InlineData(typeof(SmtpClient), "SendAsync", new[] { "Void SendAsync(System.String, System.String, System.String, System.String, System.Object)" }, true)] [InlineData(typeof(SmtpClient), "SendMailAsync", new[] { "System.Threading.Tasks.Task SendMailAsync(System.String, System.String, System.String, System.String, System.Threading.CancellationToken)", "System.Threading.Tasks.Task SendMailAsync(System.String, System.String, System.String, System.String)" }, true)] +#if NET6_0_OR_GREATER + [InlineData(typeof(DefaultInterpolatedStringHandler), null, new[] { "Void AppendFormatted(System.ReadOnlySpan`1[System.Char], Int32, System.String)" }, true)] +#endif [Trait("Category", "EndToEnd")] [Trait("RunOnWindows", "True")] public void TestMethodsAspectCover(Type typeToCheck, string methodToCheck, string[] overloadsToExclude = null, bool excludeParameterlessMethods = false) @@ -305,6 +309,9 @@ public void TestFileClassMethodsAspectCover() [InlineData(typeof(Assembly))] #endif [InlineData(typeof(Assembly), new string[] { "System.Reflection.Assembly::Load(System.String,System.Security.Policy.Evidence)" })] +#if NET6_0_OR_GREATER + [InlineData(typeof(DefaultInterpolatedStringHandler), null)] +#endif [Trait("Category", "EndToEnd")] [Trait("RunOnWindows", "True")] public void TestAllAspectsHaveACorrespondingMethod(Type type, string[] aspectsToExclude = null) @@ -389,7 +396,7 @@ private string NormalizeName(string signature) return signature.Replace(" ", string.Empty).Replace("[T]", string.Empty).Replace("", string.Empty) .Replace("[", "<").Replace("]", ">").Replace(",...", string.Empty).Replace("System.", string.Empty) - .Replace("ByRef", string.Empty); + .Replace("ByRef", string.Empty).Replace("!!0", "T"); } private bool MethodShouldBeChecked(MethodBase method) @@ -445,7 +452,13 @@ private void TestMethodOverloads(Type typeToCheck, string methodToCheck, List NormalizeName(x).Contains(methodSignature) && x.Contains(typeToCheck.FullName)); + var isCovered = aspects.Any(x => + { + var normalized = NormalizeName(x); + var contains = normalized.Contains(methodSignature); + var xcontains = x.Contains(typeToCheck.FullName); + return contains && xcontains; + }); isCovered.Should().BeTrue(method.ToString() + " is not covered"); } } diff --git a/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringHandlerTests.cs b/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringHandlerTests.cs index 652aa17848bb..4fc830c48207 100644 --- a/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringHandlerTests.cs +++ b/tracer/test/test-applications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/DefaultInterpolatedStringHandlerTests.cs @@ -245,17 +245,17 @@ public void GivenImplicitInterpolatedString_WhenAddingTaintedValuesNested_GetStr { const int number = 42; var date = new DateTime(2024, 11, 22, 15, 30, 0); - const decimal decimalValue = 123.456m; + const decimal decimalValue = 123; const bool booleanValue = true; const char charValue = 'A'; var nestedInterpolatedString1 = $"Nested1 {TaintedValue} and {number}"; var nestedInterpolatedString2 = $"Nested2 {nestedInterpolatedString1} with date {date:yyyy-MM-dd}"; - var complexString = $"Complex {nestedInterpolatedString2} and decimal {decimalValue:F2} and boolean {booleanValue}"; + var complexString = $"Complex {nestedInterpolatedString2} and decimal {decimalValue} and boolean {booleanValue}"; var nestedString = $"Hello {$"{TaintedValue + "Hello"} - {complexString}"}"; var finalString = $"Final {nestedString} and char {charValue} with additional {UntaintedValue} and number {number} and date {date:HH:mm:ss}"; - finalString.Should().Be("Final Hello Hello - Complex Nested2 Nested1 tainted and 42 with date 2024-11-22 and decimal 123.46 and boolean True and char A with additional untainted and number 42 and date 15:30:00"); + finalString.Should().Be("Final Hello taintedHello - Complex Nested2 Nested1 tainted and 42 with date 2024-11-22 and decimal 123 and boolean True and char A with additional untainted and number 42 and date 15:30:00"); AssertTainted(finalString); } @@ -263,10 +263,9 @@ public void GivenImplicitInterpolatedString_WhenAddingTaintedValuesNested_GetStr public void GivenImplicitInterpolatedString_WhenAddingTaintedValuesComplex_GetString_Vulnerable() { var interpolatedString = $""" - Hello "{TaintedValue}" and "{UntaintedValue}". - . + Hello "{TaintedValue}" and "{UntaintedValue}".. """; - interpolatedString.Should().Be("Hello \"tainted\" and \"untainted\".\n."); + interpolatedString.Should().Be("Hello \"tainted\" and \"untainted\".."); AssertTainted(interpolatedString); } }