Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ASM] iast: Tainting of DefaultInterpolatedStringHandler #6340

Merged
merged 16 commits into from
Nov 28, 2024
Merged

[ASM] iast: Tainting of DefaultInterpolatedStringHandler #6340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
2112c9a
aspects not working
e-n-0 Nov 17, 2024
ce53aaf
update
e-n-0 Nov 19, 2024
32defa8
Preliminary support for ref struct instrumentation to cover interpola…
daniel-romano-DD Nov 21, 2024
23288bd
Rebase fix
daniel-romano-DD Nov 22, 2024
826f202
Rebase fix
daniel-romano-DD Nov 22, 2024
008717e
Converted dyn method from Emit to Fody
daniel-romano-DD Nov 22, 2024
41d581c
Add update aspects and test for detection sqli interpolated string
e-n-0 Nov 22, 2024
ca238ad
update to good snapshot
e-n-0 Nov 24, 2024
8b65a31
remove useless + revert changes in native part
e-n-0 Nov 24, 2024
b9ea3d2
fix snapshot
e-n-0 Nov 24, 2024
30dfe3e
Update to set the tainted string as the result length + update marks …
e-n-0 Nov 25, 2024
e777c49
Applied comment
e-n-0 Nov 26, 2024
c7ffef0
not working aspect
e-n-0 Nov 26, 2024
2c974d2
Fixed broken generic aspects support
daniel-romano-DD Nov 27, 2024
15c9439
Add generic aspects + handle tests
e-n-0 Nov 27, 2024
bfdfd64
Add more tests
e-n-0 Nov 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion tracer/build/_build/CodeGenerators/CallSitesGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,26 @@ internal static void RetrieveCallSites(Dictionary<string, AspectClass> 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)
{
throw new InvalidOperationException("Could not find '::' in method name " + fullName);
}

return fullName.Substring(methodNameStart + 2).Replace("<T>", "<!!0>").Replace("&", "");

static string GetParameter(ParameterDefinition parameter)
{
var paramType = parameter.ParameterType.FullName;
return paramType switch
{
"T" => "!!0",
_ => paramType.Replace("<T>", "<!!0>"),
};
}
}

static bool IsAspectClass(Mono.Cecil.CustomAttribute attribute)
Expand Down
233 changes: 233 additions & 0 deletions ...r/src/Datadog.Trace/Iast/Aspects/System.Runtime/DefaultInterpolatedStringHandlerAspect.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,233 @@
// <copyright file="DefaultInterpolatedStringHandlerAspect.cs" company="Datadog">
// 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.
// </copyright>

#if NET6_0_OR_GREATER

#nullable enable

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;

namespace Datadog.Trace.Iast.Aspects.System.Runtime;

/// <summary> DefaultInterpolatedString class aspect </summary>
[AspectClass("System.Runtime")]
[global::System.ComponentModel.Browsable(false)]
[global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
public class DefaultInterpolatedStringHandlerAspect
{
/// <summary>
/// System.Runtime DefaultInterpolatedStringHandler.AppendFormatted(String) aspect
/// </summary>
/// <param name="target"> the ref DefaultInterpolatedStringHandler </param>
/// <param name="value"> the string value </param>
[AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(System.String)")]
public static void AppendFormatted1(ref DefaultInterpolatedStringHandler target, string value)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might also consider to add these overloads:

AppendFormatted(Object, Int32, String)
AppendFormatted(T, String)
AppendFormatted(T, Int32, String)

Actually, you could add unit tests in IastInstrumentationUnitTests.TestMethodsAspectCover and TestAllAspectsHaveACorrespondingMethod to make sure that you cover every method that you want to cover for every framework and that you don't have useless aspects for some frameworks

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should cover relevant string related overloads, those used by the compiler when generating code for interpolated strings

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if a string is declared as an object, we could potentially miss it. For instance, this test would fail:

[Fact]
public void GivenImplicitInterpolatedString_WhenAddingTaintedValuesComplex_GetString_Vulnerable()
{
    var interpolatedString = $"""
                              Hello "{((object)TaintedValue)}".
                              .
                              """;
    AssertTainted(interpolatedString);
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, adding these unit test could detect if we are not covering a new string method in the DefaultInterpolatedStringHandler class that could be included in future versions of .Net

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😄

  • I applied in new commits the supports of this test case
  • I also added the tests in the IastInstrumentationUnitTests class for the aspects checks

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thank you!

{
target.AppendFormatted(value);
try
{
DefaultInterpolatedStringHandlerModuleImpl.Append(ToPointer(ref target), value);
}
catch (Exception ex)
{
IastModule.Log.Warning(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted1)}");
}
}

/// <summary>
/// System.Runtime DefaultInterpolatedStringHandler.AppendFormatted(String, Int32, String) aspect
/// </summary>
/// <param name="target"> the ref DefaultInterpolatedStringHandler </param>
/// <param name="value"> the string value </param>
/// <param name="alignment"> the alignment value </param>
/// <param name="format"> the format value </param>
[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)}");
}
}

/// <summary>
/// System.Runtime DefaultInterpolatedStringHandler.AppendFormatted(Object, Int32, String) aspect
/// </summary>
/// <param name="target"> the ref DefaultInterpolatedStringHandler </param>
/// <param name="value"> the object value </param>
/// <param name="alignment"> the alignment value </param>
/// <param name="format"> the format value </param>
[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)}");
}
}

/// <summary>
/// System.Runtime DefaultInterpolatedStringHandler.AppendFormatted(T) aspect
/// </summary>
/// <typeparam name="T">The first generic type parameter.</typeparam>
/// <param name="target"> the ref DefaultInterpolatedStringHandler </param>
/// <param name="value"> the string value </param>
[AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0)")]
public static void AppendFormatted4<T>(ref DefaultInterpolatedStringHandler target, T value)
{
target.AppendFormatted<T>(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)}");
}
}

/// <summary>
/// System.Runtime DefaultInterpolatedStringHandler.AppendFormatted(T, String) aspect
/// </summary>
/// <typeparam name="T">The first generic type parameter.</typeparam>
/// <param name="target"> the ref DefaultInterpolatedStringHandler </param>
/// <param name="value"> the string value </param>
/// <param name="alignment"> the alignment value </param>
[AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0,System.Int32)")]
public static void AppendFormatted5<T>(ref DefaultInterpolatedStringHandler target, T value, int alignment)
{
target.AppendFormatted(value, alignment);
try
{
if (value is string str)
{
DefaultInterpolatedStringHandlerModuleImpl.Append(ToPointer(ref target), str);
}
}
catch (Exception ex)
{
IastModule.Log.Warning(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendFormatted5)}");
}
}

/// <summary>
/// System.Runtime DefaultInterpolatedStringHandler.AppendFormatted(T, String) aspect
/// </summary>
/// <typeparam name="T">The first generic type parameter.</typeparam>
/// <param name="target"> the ref DefaultInterpolatedStringHandler </param>
/// <param name="value"> the string value </param>
/// <param name="format"> the format value </param>
[AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0,System.String)")]
public static void AppendFormatted6<T>(ref DefaultInterpolatedStringHandler target, T value, string? format)
{
target.AppendFormatted(value, 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(AppendFormatted6)}");
}
}

/// <summary>
/// System.Runtime DefaultInterpolatedStringHandler.AppendFormatted(T, String) aspect
/// </summary>
/// <typeparam name="T">The first generic type parameter.</typeparam>
/// <param name="target"> the ref DefaultInterpolatedStringHandler </param>
/// <param name="value"> the string value </param>
/// <param name="alignment"> the alignment value </param>
/// <param name="format"> the format value </param>
[AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::AppendFormatted(!!0,System.Int32,System.String)")]
public static void AppendFormatted7<T>(ref DefaultInterpolatedStringHandler target, T 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(AppendFormatted7)}");
}
}

/// <summary>
/// System.Runtime DefaultInterpolatedStringHandler.AppendLiteral(String) aspect
/// </summary>
/// <param name="target"> the ref DefaultInterpolatedStringHandler </param>
/// <param name="value"> the string value </param>
[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.Warning(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(AppendLiteral)}");
}
}

/// <summary>
/// System.Runtime DefaultInterpolatedStringHandler.ToStringAndClear aspect
/// </summary>
/// <param name="target"> the ref DefaultInterpolatedStringHandler </param>
/// <returns> the string value </returns>
[AspectMethodReplace("System.Runtime.CompilerServices.DefaultInterpolatedStringHandler::ToStringAndClear()")]
public static string ToStringAndClear(ref DefaultInterpolatedStringHandler target)
{
var result = target.ToStringAndClear();
try
{
DefaultInterpolatedStringHandlerModuleImpl.PropagateTaint(ToPointer(ref target), result);
}
catch (Exception ex)
{
IastModule.Log.Warning(ex, $"Error invoking {nameof(DefaultInterpolatedStringHandlerAspect)}.{nameof(ToStringAndClear)}");
}

return result;
}

private static IntPtr ToPointer(ref DefaultInterpolatedStringHandler ts)
{
Ldarg(nameof(ts));
return IL.Return<IntPtr>();
}
}

#endif
15 changes: 13 additions & 2 deletions tracer/src/Datadog.Trace/Iast/DefaultTaintedMap.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
110 changes: 110 additions & 0 deletions tracer/src/Datadog.Trace/Iast/Propagation/DefaultInterpolatedStringHandlerModuleImpl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
// <copyright file="DefaultInterpolatedStringHandlerModuleImpl.cs" company="Datadog">
// 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.
// </copyright>

#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 unsafe void Append(IntPtr target, string? value)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could consider adding some logic to keep the ranges in some methods such as append without format (like we do with strings). Anyway, this could be done in a separate PR and we should evaluate also the performance cost, so maybe it's not worth it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean like keeping correct start and length of tainted data in the ranges?
Currently it's difficult to know the current length before appending the new data to the DefaultInterpolatedStringHandler. The _pos int of the struct is private. 😞
We could get the value by knowing the exact offset in the struct but that would be unsafe (and not done for this PR atm).

{
FullTaintIfAnyTainted(target, value);
}

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)
{
return;
}

var taintedObjects = iastContext.GetTaintedObjects();
var tainted = PropagationModuleImpl.GetTainted(taintedObjects, target);
var targetIsTainted = tainted is not null;

if (!targetIsTainted && (tainted = GetTaintedWithRanges(taintedObjects, input)) is null)
{
return;
}

var rangesResult = new[] { new Range(0, 0, tainted!.Ranges[0].Source, tainted.Ranges[0].SecureMarks) };
if (!targetIsTainted)
{
taintedObjects.Taint(target, rangesResult);
}
else
{
tainted.Ranges = rangesResult;
}
}
catch (Exception error)
{
IastModule.Log.Error(error, $"{nameof(DefaultInterpolatedStringHandlerModuleImpl)}.{nameof(FullTaintIfAnyTainted)} exception");
}
}

public static object? PropagateTaint(object? input, string? result)
{
try
{
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;
}

var range = new Range(0, result.Length, taintedSelf.Ranges[0].Source, taintedSelf.Ranges[0].SecureMarks);
taintedObjects.Taint(result, [range]);
}
catch (Exception err)
{
IastModule.Log.Error(err, $"{nameof(DefaultInterpolatedStringHandlerModuleImpl)}.{nameof(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;
}
}

#endif
Loading
Loading