Fixed Join with char separator bug

DataDog · Nov 1, 2023 · 0dcc1e3 · 0dcc1e3
1 parent 8398901
commit 0dcc1e3
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 12 deletions.
diff --git a/...net461/Datadog.Trace.SourceGenerators/AspectsDefinitionsGenerator/AspectsDefinitions.g.cs b/...net461/Datadog.Trace.SourceGenerators/AspectsDefinitionsGenerator/AspectsDefinitions.g.cs
@@ -363,8 +363,8 @@ internal static partial class AspectDefinitions
 "  [AspectMethodReplace(\"System.String::Join(System.String,System.String[],System.Int32,System.Int32)\",\"\",[0],[False],[None],Propagation,[])] Join(System.String,System.String[],System.Int32,System.Int32)",
 "  [AspectMethodReplace(\"System.String::Join(System.String,System.Object[])\",\"\",[0],[False],[None],Propagation,[])] Join(System.String,System.Object[])",
 "  [AspectMethodReplace(\"System.String::Join(System.String,System.String[])\",\"\",[0],[False],[None],Propagation,[])] Join(System.String,System.String[])",
-"  [AspectMethodReplace(\"System.String::Join(System.String,System.Collections.Generic.IEnumerable`1<System.String>)\",\"\",[0],[False],[None],Propagation,[])] Join(System.String,System.Collections.IEnumerable)",
-"  [AspectMethodReplace(\"System.String::Join(System.String,System.Collections.Generic.IEnumerable`1<!!0>)\",\"\",[0],[False],[None],Propagation,[])] Join2(System.String,System.Collections.IEnumerable)",
+"  [AspectMethodReplace(\"System.String::Join(System.String,System.Collections.Generic.IEnumerable`1<System.String>)\",\"\",[0],[False],[None],Propagation,[])] JoinString(System.String,System.Collections.IEnumerable)",
+"  [AspectMethodReplace(\"System.String::Join(System.String,System.Collections.Generic.IEnumerable`1<!!0>)\",\"\",[0],[False],[None],Propagation,[])] Join(System.String,System.Collections.IEnumerable)",
 "  [AspectMethodReplace(\"System.String::ToUpper()\",\"\",[0],[False],[StringLiteral_0],Propagation,[])] ToUpper(System.String)",
 "  [AspectMethodReplace(\"System.String::ToUpper(System.Globalization.CultureInfo)\",\"\",[0],[False],[StringLiteral_0],Propagation,[])] ToUpper(System.String,System.Globalization.CultureInfo)",
 "  [AspectMethodReplace(\"System.String::ToUpperInvariant()\",\"\",[0],[False],[StringLiteral_0],Propagation,[])] ToUpperInvariant(System.String)",

diff --git a/...net6.0/Datadog.Trace.SourceGenerators/AspectsDefinitionsGenerator/AspectsDefinitions.g.cs b/...net6.0/Datadog.Trace.SourceGenerators/AspectsDefinitionsGenerator/AspectsDefinitions.g.cs
@@ -405,8 +405,8 @@ internal static partial class AspectDefinitions
 "  [AspectMethodReplace(\"System.String::Join(System.Char,System.Collections.Generic.IEnumerable`1<!!0>)\",\"\",[0],[False],[None],Propagation,[])] Join(System.Char,System.Collections.IEnumerable)",
 "  [AspectMethodReplace(\"System.String::Join(System.String,System.Object[])\",\"\",[0],[False],[None],Propagation,[])] Join(System.String,System.Object[])",
 "  [AspectMethodReplace(\"System.String::Join(System.String,System.String[])\",\"\",[0],[False],[None],Propagation,[])] Join(System.String,System.String[])",
-"  [AspectMethodReplace(\"System.String::Join(System.String,System.Collections.Generic.IEnumerable`1<System.String>)\",\"\",[0],[False],[None],Propagation,[])] Join(System.String,System.Collections.IEnumerable)",
-"  [AspectMethodReplace(\"System.String::Join(System.String,System.Collections.Generic.IEnumerable`1<!!0>)\",\"\",[0],[False],[None],Propagation,[])] Join2(System.String,System.Collections.IEnumerable)",
+"  [AspectMethodReplace(\"System.String::Join(System.String,System.Collections.Generic.IEnumerable`1<System.String>)\",\"\",[0],[False],[None],Propagation,[])] JoinString(System.String,System.Collections.IEnumerable)",
+"  [AspectMethodReplace(\"System.String::Join(System.String,System.Collections.Generic.IEnumerable`1<!!0>)\",\"\",[0],[False],[None],Propagation,[])] Join(System.String,System.Collections.IEnumerable)",
 "  [AspectMethodReplace(\"System.String::ToUpper()\",\"\",[0],[False],[StringLiteral_0],Propagation,[])] ToUpper(System.String)",
 "  [AspectMethodReplace(\"System.String::ToUpper(System.Globalization.CultureInfo)\",\"\",[0],[False],[StringLiteral_0],Propagation,[])] ToUpper(System.String,System.Globalization.CultureInfo)",
 "  [AspectMethodReplace(\"System.String::ToUpperInvariant()\",\"\",[0],[False],[StringLiteral_0],Propagation,[])] ToUpperInvariant(System.String)",

diff --git a/...app3.1/Datadog.Trace.SourceGenerators/AspectsDefinitionsGenerator/AspectsDefinitions.g.cs b/...app3.1/Datadog.Trace.SourceGenerators/AspectsDefinitionsGenerator/AspectsDefinitions.g.cs
@@ -392,8 +392,8 @@ internal static partial class AspectDefinitions
 "  [AspectMethodReplace(\"System.String::Join(System.Char,System.Collections.Generic.IEnumerable`1<!!0>)\",\"\",[0],[False],[None],Propagation,[])] Join(System.Char,System.Collections.IEnumerable)",
 "  [AspectMethodReplace(\"System.String::Join(System.String,System.Object[])\",\"\",[0],[False],[None],Propagation,[])] Join(System.String,System.Object[])",
 "  [AspectMethodReplace(\"System.String::Join(System.String,System.String[])\",\"\",[0],[False],[None],Propagation,[])] Join(System.String,System.String[])",
-"  [AspectMethodReplace(\"System.String::Join(System.String,System.Collections.Generic.IEnumerable`1<System.String>)\",\"\",[0],[False],[None],Propagation,[])] Join(System.String,System.Collections.IEnumerable)",
-"  [AspectMethodReplace(\"System.String::Join(System.String,System.Collections.Generic.IEnumerable`1<!!0>)\",\"\",[0],[False],[None],Propagation,[])] Join2(System.String,System.Collections.IEnumerable)",
+"  [AspectMethodReplace(\"System.String::Join(System.String,System.Collections.Generic.IEnumerable`1<System.String>)\",\"\",[0],[False],[None],Propagation,[])] JoinString(System.String,System.Collections.IEnumerable)",
+"  [AspectMethodReplace(\"System.String::Join(System.String,System.Collections.Generic.IEnumerable`1<!!0>)\",\"\",[0],[False],[None],Propagation,[])] Join(System.String,System.Collections.IEnumerable)",
 "  [AspectMethodReplace(\"System.String::ToUpper()\",\"\",[0],[False],[StringLiteral_0],Propagation,[])] ToUpper(System.String)",
 "  [AspectMethodReplace(\"System.String::ToUpper(System.Globalization.CultureInfo)\",\"\",[0],[False],[StringLiteral_0],Propagation,[])] ToUpper(System.String,System.Globalization.CultureInfo)",
 "  [AspectMethodReplace(\"System.String::ToUpperInvariant()\",\"\",[0],[False],[StringLiteral_0],Propagation,[])] ToUpperInvariant(System.String)",

diff --git a/...ard2.0/Datadog.Trace.SourceGenerators/AspectsDefinitionsGenerator/AspectsDefinitions.g.cs b/...ard2.0/Datadog.Trace.SourceGenerators/AspectsDefinitionsGenerator/AspectsDefinitions.g.cs
@@ -382,8 +382,8 @@ internal static partial class AspectDefinitions
 "  [AspectMethodReplace(\"System.String::Join(System.Char,System.Collections.Generic.IEnumerable`1<!!0>)\",\"\",[0],[False],[None],Propagation,[])] Join(System.Char,System.Collections.IEnumerable)",
 "  [AspectMethodReplace(\"System.String::Join(System.String,System.Object[])\",\"\",[0],[False],[None],Propagation,[])] Join(System.String,System.Object[])",
 "  [AspectMethodReplace(\"System.String::Join(System.String,System.String[])\",\"\",[0],[False],[None],Propagation,[])] Join(System.String,System.String[])",
-"  [AspectMethodReplace(\"System.String::Join(System.String,System.Collections.Generic.IEnumerable`1<System.String>)\",\"\",[0],[False],[None],Propagation,[])] Join(System.String,System.Collections.IEnumerable)",
-"  [AspectMethodReplace(\"System.String::Join(System.String,System.Collections.Generic.IEnumerable`1<!!0>)\",\"\",[0],[False],[None],Propagation,[])] Join2(System.String,System.Collections.IEnumerable)",
+"  [AspectMethodReplace(\"System.String::Join(System.String,System.Collections.Generic.IEnumerable`1<System.String>)\",\"\",[0],[False],[None],Propagation,[])] JoinString(System.String,System.Collections.IEnumerable)",
+"  [AspectMethodReplace(\"System.String::Join(System.String,System.Collections.Generic.IEnumerable`1<!!0>)\",\"\",[0],[False],[None],Propagation,[])] Join(System.String,System.Collections.IEnumerable)",
 "  [AspectMethodReplace(\"System.String::ToUpper()\",\"\",[0],[False],[StringLiteral_0],Propagation,[])] ToUpper(System.String)",
 "  [AspectMethodReplace(\"System.String::ToUpper(System.Globalization.CultureInfo)\",\"\",[0],[False],[StringLiteral_0],Propagation,[])] ToUpper(System.String,System.Globalization.CultureInfo)",
 "  [AspectMethodReplace(\"System.String::ToUpperInvariant()\",\"\",[0],[False],[StringLiteral_0],Propagation,[])] ToUpperInvariant(System.String)",

diff --git a/tracer/src/Datadog.Trace/Iast/Aspects/System/StringAspects.cs b/tracer/src/Datadog.Trace/Iast/Aspects/System/StringAspects.cs
@@ -415,7 +415,7 @@ public static string Join(char separator, string[] values)
     [AspectMethodReplace("System.String::Join(System.Char,System.Object[])")]
     public static string Join(char separator, object[] values)
     {
-        return OnStringJoin(string.Join(separator.ToString(), values), values);
+        return Join(separator.ToString(), values);
     }
 
     /// <summary>
@@ -476,7 +476,7 @@ public static string Join(string separator, string[] values)
     /// <param name="values"> values to join </param>
     /// <returns> Join result </returns>
     [AspectMethodReplace("System.String::Join(System.String,System.Collections.Generic.IEnumerable`1<System.String>)")]
-    public static string Join(string separator, IEnumerable values)
+    public static string JoinString(string separator, IEnumerable values)
     {
         if (values is null)
         {
@@ -503,7 +503,7 @@ public static string Join(string separator, IEnumerable values)
     /// <param name="values"> values to join </param>
     /// <returns> Join result </returns>
     [AspectMethodReplace("System.String::Join(System.String,System.Collections.Generic.IEnumerable`1<!!0>)")]
-    public static string Join2(string separator, IEnumerable values)
+    public static string Join(string separator, IEnumerable values)
     {
         if (values is null)
         {
@@ -517,7 +517,6 @@ public static string Join2(string separator, IEnumerable values)
         }
 
         // We have a IEnumerable of structs or basic types. This is a corner case.
-
         try
         {
             valuesConverted = values.Cast<object>();

diff --git a/...ications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/StringJoinTests.cs b/...ications/integrations/Samples.InstrumentedTests/Vulnerabilities/String/StringJoinTests.cs
@@ -249,7 +249,37 @@ public void GivenATaintedStringInClass_WhenCallingJoin_ResultIsTainted5()
         AssertTaintedFormatWithOriginalCallCheck(":+-tainted-+:", String.Join(",", new ClassForStringTest(taintedValue)), () => String.Join(",", new ClassForStringTest(taintedValue)));
     }
 
+
 #if !NET462
+    [Fact]
+    public void GivenATaintedStringInNestedMethodObject_WhenCallingJoinWithChar_ResultIsTainted6()
+    {
+        void NestedMethod<T>(List<T> parameters)
+        {
+            AssertTaintedFormatWithOriginalCallCheck(":+-tainted-+:", String.Join(',', parameters), () => String.Join(',', parameters));
+        }
+
+        NestedMethod(new List<string> { taintedValue });
+    }
+
+    [Fact]
+    public void GivenATaintedStringInNestedMethodObject_WhenCallingJoinWithChar_ResultIsTainted7()
+    {
+        void NestedMethod<T>(List<T> parameters)
+        {
+            AssertTaintedFormatWithOriginalCallCheck(":+-tainted-+:,NonTainted", String.Join(',', parameters), () => String.Join(',', parameters));
+        }
+
+        NestedMethod(new List<string> { taintedValue, "NonTainted" });
+    }
+
+    [Fact]
+    public void GivenATaintedStringInList_WhenCallingJoinWithChar_ResultIsTainted8()
+    {
+        var parameters = new List<string> { taintedValue, "NonTainted" };
+        AssertTaintedFormatWithOriginalCallCheck(":+-tainted-+:,NonTainted", String.Join(',', parameters), () => String.Join(',', parameters));
+    }
+
     [Fact]
     public void GivenATaintedString_WhenCallingJoin_ResultIsTainted7()
     {