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

[IAST] Fix flaky Interpolated Strings tests #6555

Merged
merged 12 commits into from
Jan 31, 2025
Merged

[IAST] Fix flaky Interpolated Strings tests #6555

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
2e78e71
Fix flaky tests
daniel-romano-DD Jan 15, 2025
126c73b
Added thread static deque to keep alive ref struct boxed objects if t…
daniel-romano-DD Jan 29, 2025
982f965
invalidate tainted object when string returned
e-n-0 Jan 29, 2025
25e59a7
PR Comments
daniel-romano-DD Jan 29, 2025
7f26b51
Safeguard improvement
daniel-romano-DD Jan 29, 2025
dd6e7ae
Fixed tests
daniel-romano-DD Jan 29, 2025
b7fa57c
Fix snapshot
daniel-romano-DD Jan 30, 2025
4dca993
Minor tweaks
daniel-romano-DD Jan 30, 2025
7961f18
Tweaked stack size
daniel-romano-DD Jan 30, 2025
58f539c
Fixed variable local
daniel-romano-DD Jan 30, 2025
4b018f0
Fix local
daniel-romano-DD Jan 30, 2025
b28b996
Fix thread static var access and init
e-n-0 Jan 30, 2025
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
2 changes: 2 additions & 0 deletions tracer/src/Datadog.Trace/Iast/ITaintedObject.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,6 @@ internal interface ITaintedObject
public ITaintedObject? Next { get; set; }

public int PositiveHashCode { get; }

public void Invalidate();
}
17 changes: 16 additions & 1 deletion tracer/src/Datadog.Trace/Iast/Propagation/DefaultInterpolatedStringHandlerModuleImpl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#nullable enable

using System;
using System.Collections.Generic;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Text;
Expand All @@ -17,6 +18,9 @@ namespace Datadog.Trace.Iast.Propagation;

internal static class DefaultInterpolatedStringHandlerModuleImpl
{
[ThreadStatic]
private static Queue<object> _taintedRefStructs = new(); // Keep alive the tainted ref structs
daniel-romano-DD marked this conversation as resolved.
Show resolved Hide resolved

public static unsafe void Append(IntPtr target, string? value)
{
FullTaintIfAnyTainted(target, value);
Expand Down Expand Up @@ -51,7 +55,16 @@ public static unsafe void FullTaintIfAnyTainted(IntPtr target, string? input)
var rangesResult = new[] { new Range(0, 0, tainted!.Ranges[0].Source, tainted.Ranges[0].SecureMarks) };
if (!targetIsTainted)
{
taintedObjects.Taint(target, rangesResult);
object targetObj = target;
_taintedRefStructs.Enqueue(targetObj);

// Safe guard to avoid memory leak
while (_taintedRefStructs.Count > 20)
{
_taintedRefStructs.Dequeue();
}

taintedObjects.Taint(targetObj, rangesResult);
}
else
{
Expand Down Expand Up @@ -91,6 +104,8 @@ public static unsafe void FullTaintIfAnyTainted(IntPtr target, string? input)

var range = new Range(0, result.Length, taintedSelf.Ranges[0].Source, taintedSelf.Ranges[0].SecureMarks);
taintedObjects.Taint(result, [range]);
_taintedRefStructs.Dequeue();
taintedSelf.Invalidate();
daniel-romano-DD marked this conversation as resolved.
Show resolved Hide resolved
}
catch (Exception err)
{
Expand Down
5 changes: 5 additions & 0 deletions tracer/src/Datadog.Trace/Iast/TaintedObject.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,9 @@ public TaintedObject(object value, Range[] ranges)
public Range[] Ranges { get; set; }

public ITaintedObject? Next { get; set; }

public void Invalidate()
{
_weak.Target = null;
}
}
27 changes: 21 additions & 6 deletions ...ons/Samples.InstrumentedTests/Propagation/String/DefaultInterpolatedStringHandlerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,22 @@ public void GivenAnExplicitInterpolatedString_WhenAddingTaintedValueMultipleValu
AssertTainted(test.ToStringAndClear());
}

[Fact (Skip = "Flaky test")]
[Fact]
public void GivenAnExplicitInterpolatedString_WhenAddingTaintedValuesAndGCCollects_GetString_Vulnerable()
{
var test = new DefaultInterpolatedStringHandler();
test.AppendLiteral(UntaintedValue);
test.AppendFormatted(new ReadOnlySpan<char>([' ', 'w', 'o', 'r', 'l', 'd', ' ']));
test.AppendFormatted(TaintedValue);
test.AppendFormatted(42);

GC.Collect();

AssertTainted(test.ToStringAndClear());
}


[Fact]
public void GivenAnImplicitInterpolatedString_WhenAddingTaintedValue_GetString_Vulnerable()
{
var number = 5;
Expand All @@ -192,7 +207,7 @@ public void GivenAnImplicitInterpolatedString_WhenAddingTaintedValue_GetString_V
AssertTainted(str);
}

[Fact (Skip = "Flaky test")]
[Fact]
public void GivenAnImplicitInterpolatedString_WhenAddingUntaintedValue_GetString_NonVulnerable()
{
var number = 5;
Expand All @@ -201,7 +216,7 @@ public void GivenAnImplicitInterpolatedString_WhenAddingUntaintedValue_GetString
AssertNotTainted(str);
}

[Fact (Skip = "Flaky test")]
[Fact]
public void GivenAnImplicitInterpolatedString_WhenAddingTaintedValueAsObject_GetString_Vulnerable()
{
var number = 5;
Expand All @@ -210,7 +225,7 @@ public void GivenAnImplicitInterpolatedString_WhenAddingTaintedValueAsObject_Get
AssertTainted(str);
}

[Fact (Skip = "Flaky test")]
[Fact]
public void GivenAnImplicitInterpolatedString_WhenAddingMultipleValuesWithTaintedValues_GetString_Vulnerable()
{
var order = new
Expand Down Expand Up @@ -240,7 +255,7 @@ public void GivenAnImplicitInterpolatedString_WhenAddingMultipleValuesWithTainte
AssertTainted(sql);
}

[Fact (Skip = "Flaky test")]
[Fact]
public void GivenImplicitInterpolatedString_WhenAddingTaintedValuesNested_GetString_Vulnerable()
{
const int number = 42;
Expand All @@ -259,7 +274,7 @@ public void GivenImplicitInterpolatedString_WhenAddingTaintedValuesNested_GetStr
AssertTainted(finalString);
}

[Fact (Skip = "Flaky test")]
[Fact]
public void GivenImplicitInterpolatedString_WhenAddingTaintedValuesComplex_GetString_Vulnerable()
{
var interpolatedString = $"""
Expand Down
Loading