Skip to content

Commit 68a09e2

Browse files
committed
Attempt to fix / simulate NRE in TaintedObject Get
1 parent 1d92695 commit 68a09e2

File tree

2 files changed

+63
-0
lines changed

2 files changed

+63
-0
lines changed

tracer/test/Datadog.Trace.Security.Unit.Tests/IAST/Tainted/DefaultTaintedMapTests.cs

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,35 @@ public void GivenATaintedObjectMap_WhenDisposedInSameHashPosition_IsPurged(int t
329329
}
330330
}
331331

332+
[Fact]
333+
public void GivenATaintedObjectMap_WhenObjectEqualsThrowsOnNull_GetDoesNotThrow()
334+
{
335+
// Verifies fix for bug where customer's Equals() throws NRE on null.
336+
// Old code: objectToFind.Equals(entry.Value) where entry.Value can be null from GC'd WeakReference
337+
// Fix: ReferenceEquals() never calls customer code
338+
DefaultTaintedMap map = new();
339+
340+
var gcObject = new ObjectWithProblematicEquals("gc-target");
341+
var aliveObject = new ObjectWithProblematicEquals("alive");
342+
343+
// Force hash collision to create a chain
344+
var aliveObjectHash = IastUtils.IdentityHashCode(aliveObject) & DefaultTaintedMap.PositiveMask;
345+
346+
var aliveTainted = new TaintedForTest(aliveObject, null);
347+
map.Put(aliveTainted);
348+
349+
// gcObject becomes HEAD of chain, then gets GC'd (Value becomes null)
350+
var gcTainted = new TaintedForTest(gcObject, null);
351+
gcTainted.PositiveHashCode = aliveObjectHash;
352+
map.Put(gcTainted);
353+
gcTainted.SimulateGarbageCollection();
354+
355+
// Old code would throw NRE when walking chain encounters null Value
356+
var result = map.Get(aliveObject);
357+
result.Should().NotBeNull();
358+
result.Value.Should().Be(aliveObject);
359+
}
360+
332361
private static void AssertNotContained(DefaultTaintedMap map, List<string> objects)
333362
{
334363
foreach (var item in objects)
@@ -358,4 +387,29 @@ private static void AssertFlatMode(DefaultTaintedMap map, List<string> objects)
358387

359388
map.IsFlat.Should().BeTrue();
360389
}
390+
391+
// Test helper class that mimics customer code with a problematic Equals() implementation
392+
private class ObjectWithProblematicEquals
393+
{
394+
public ObjectWithProblematicEquals(string id)
395+
{
396+
Id = id;
397+
}
398+
399+
public string Id { get; }
400+
401+
public override bool Equals(object obj)
402+
{
403+
// This mimics customer code that doesn't handle null properly
404+
// Before the fix, this would throw NullReferenceException when
405+
// DefaultTaintedMap.Get() called objectToFind.Equals(entry.Value)
406+
// where entry.Value is null (from a garbage-collected WeakReference)
407+
return this.Id == ((ObjectWithProblematicEquals)obj).Id;
408+
}
409+
410+
public override int GetHashCode()
411+
{
412+
return Id.GetHashCode();
413+
}
414+
}
361415
}

tracer/test/Datadog.Trace.Security.Unit.Tests/IAST/Tainted/TaintedForTest.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,15 @@ public void Invalidate()
4242
_alive = false;
4343
}
4444

45+
/// <summary>
46+
/// Simulates garbage collection by setting Value to null, like WeakReference.Target
47+
/// </summary>
48+
public void SimulateGarbageCollection()
49+
{
50+
_alive = false;
51+
_value = null;
52+
}
53+
4554
internal Range[]? GetRanges()
4655
{
4756
return _ranges;

0 commit comments

Comments
 (0)