C#: Adding synthetic implicit ToString calls in binary expressions.

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expression.cs

@@ -129,7 +129,7 @@ public static void CreateDeferred(Context cx, ExpressionSyntax node, IExpression
                 cx.PopulateLater(() => Create(cx, node, parent, child));
         }
 
-        private static bool ContainsPattern(SyntaxNode node) =>
+        protected static bool ContainsPattern(SyntaxNode node) =>
             node is PatternSyntax || node is VariableDesignationSyntax || node.ChildNodes().Any(ContainsPattern);
 
         /// <summary>

csharp/extractor/Semmle.Extraction.CSharp/Entities/ExpressionNodeInfo.cs

@@ -129,7 +129,13 @@ public Location Location
 
         public ExprKind Kind { get; set; } = ExprKind.UNKNOWN;
 
-        public bool IsCompilerGenerated { get; set; }
+        public bool IsCompilerGenerated { get; init; }
+
+        /// <summary>
+        /// Whether the expression should have a compiler generated `ToString` call added,
+        /// if there is no suitable implicit cast.
+        /// </summary>
+        public bool ImplicitToString { get; private set; }
 
         public ExpressionNodeInfo SetParent(IExpressionParentEntity parent, int child)
         {
@@ -157,6 +163,12 @@ public ExpressionNodeInfo SetNode(ExpressionSyntax node)
             return this;
         }
 
+        public ExpressionNodeInfo SetImplicitToString(bool value)
+        {
+            ImplicitToString = value;
+            return this;
+        }
+
         private SymbolInfo cachedSymbolInfo;
 
         public SymbolInfo SymbolInfo

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Binary.cs

@@ -14,11 +14,35 @@ private Binary(ExpressionNodeInfo info)
 
         public static Expression Create(ExpressionNodeInfo info) => new Binary(info).TryPopulate();
 
+        private Expression CreateChild(Context cx, ExpressionSyntax node, int child)
+        {
+            // If this is a "+" expression we might need to wrap the child expressions
+            // in ToString calls
+            return Kind == ExprKind.ADD
+                ? ImplicitToString.Create(cx, node, this, child)
+                : Create(cx, node, this, child);
+        }
+
+        /// <summary>
+        /// Creates an expression from a syntax node.
+        /// Inserts type conversion as required.
+        /// Population is deferred to avoid overflowing the stack.
+        /// </summary>
+        private void CreateDeferred(Context cx, ExpressionSyntax node, int child)
+        {
+            if (ContainsPattern(node))
+                // Expressions with patterns should be created right away, as they may introduce
+                // local variables referenced in `LocalVariable::GetAlreadyCreated()`
+                CreateChild(cx, node, child);
+            else
+                cx.PopulateLater(() => CreateChild(cx, node, child));
+        }
+
         protected override void PopulateExpression(TextWriter trapFile)
         {
             OperatorCall(trapFile, Syntax);
-            CreateDeferred(Context, Syntax.Left, this, 0);
-            CreateDeferred(Context, Syntax.Right, this, 1);
+            CreateDeferred(Context, Syntax.Left, 0);
+            CreateDeferred(Context, Syntax.Right, 1);
         }
 
         private static ExprKind GetKind(Context cx, BinaryExpressionSyntax node)

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/ImplicitCast.cs

@@ -156,6 +156,12 @@ convertedType.Symbol is IPointerTypeSymbol &&
                 return new ImplicitCast(info);
             }
 
+            if (info.ImplicitToString)
+            {
+                // x -> x.ToString() in "abc" + x
+                return ImplicitToString.Wrap(info);
+            }
+
             // Default: Just create the expression without a conversion.
             return Factory.Create(info);
         }

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/ImplicitToString.cs

@@ -0,0 +1,59 @@
+using System.Linq;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Semmle.Extraction.CSharp.Util;
+using Semmle.Extraction.Kinds;
+
+
+namespace Semmle.Extraction.CSharp.Entities.Expressions
+{
+    internal sealed class ImplicitToString : Expression
+    {
+        /// <summary>
+        /// Gets the `ToString` method for the given type.
+        /// </summary>
+        private static IMethodSymbol? GetToStringMethod(ITypeSymbol? type)
+        {
+            return type?
+                .GetMembers()
+                .OfType<IMethodSymbol>()
+                .Where(method =>
+                    method.GetName() == "ToString" &&
+                    method.Parameters.Length == 0
+                )
+                .FirstOrDefault();
+        }
+
+        private ImplicitToString(ExpressionNodeInfo info, IMethodSymbol toString) : base(new ExpressionInfo(info.Context, info.ConvertedType, info.Location, ExprKind.METHOD_INVOCATION, info.Parent, info.Child, isCompilerGenerated: true, info.ExprValue))
+        {
+            Factory.Create(info.SetParent(this, -1));
+
+            var target = Method.Create(Context, toString);
+            Context.TrapWriter.Writer.expr_call(this, target);
+        }
+
+        private static bool IsStringType(AnnotatedTypeSymbol? type) =>
+            type.HasValue && type.Value.Symbol?.SpecialType == SpecialType.System_String;
+
+        /// <summary>
+        /// Creates a new expression, adding a compiler generated `ToString` call if required.
+        /// </summary>
+        public static Expression Create(Context cx, ExpressionSyntax node, Expression parent, int child)
+        {
+            var info = new ExpressionNodeInfo(cx, node, parent, child);
+            return CreateFromNode(info.SetImplicitToString(IsStringType(parent.Type) && !IsStringType(info.Type)));
+        }
+
+        /// <summary>
+        /// Wraps the resulting expression in a `ToString` call, if a suitable `ToString` method is available.
+        /// </summary>
+        public static Expression Wrap(ExpressionNodeInfo info)
+        {
+            if (GetToStringMethod(info.ConvertedType.Symbol) is IMethodSymbol toString)
+            {
+                return new ImplicitToString(info, toString);
+            }
+            return Factory.Create(info);
+        }
+    }
+}

csharp/ql/examples/snippets/ternary_conditional.ql

@@ -11,7 +11,7 @@ import csharp
 
 from ConditionalExpr e
 where
-  e.getThen().stripImplicitCasts() != e.getElse().stripImplicitCasts() and
+  e.getThen().stripImplicit() != e.getElse().stripImplicit() and
   not e.getThen().getType() instanceof NullType and
   not e.getElse().getType() instanceof NullType
 select e

csharp/ql/lib/change-notes/2025-01-09-implicit-to-string.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added extractor support for extracting implicit `ToString` calls in binary `+` expressions.

csharp/ql/lib/semmle/code/csharp/PrintAst.qll

@@ -32,7 +32,9 @@ private predicate shouldPrint(Element e, Location l) {
 }
 
 private predicate isImplicitExpression(ControlFlowElement element) {
-  element.(Expr).isImplicit() and
+  // Include compiler generated cast expressions and `ToString` calls if
+  // they wrap actual source expressions.
+  element.(Expr).stripImplicit().isImplicit() and
   not element instanceof CastExpr and
   not element.(OperatorCall).getTarget() instanceof ImplicitConversionOperator and
   not element instanceof ElementInitializer

csharp/ql/lib/semmle/code/csharp/commons/Constants.qll

@@ -43,7 +43,7 @@ predicate isConstantComparison(ComparisonOperation co, boolean b) {
 private module ConstantComparisonOperation {
   private import semmle.code.csharp.commons.ComparisonTest
 
-  private SimpleType convertedType(Expr expr) { result = expr.stripImplicitCasts().getType() }
+  private SimpleType convertedType(Expr expr) { result = expr.stripImplicit().getType() }
 
   private int maxValue(Expr expr) {
     if convertedType(expr) instanceof IntegralType and exists(expr.getValue())

csharp/ql/lib/semmle/code/csharp/commons/Strings.qll

@@ -44,8 +44,8 @@ class ImplicitToStringExpr extends Expr {
     )
     or
     exists(AddExpr add, Expr o | o = add.getAnOperand() |
-      o.stripImplicitCasts().getType() instanceof StringType and
-      this = add.getOtherOperand(o)
+      o.stripImplicit().getType() instanceof StringType and
+      this = add.getOtherOperand(o).stripImplicit()
     )
     or
     this = any(InterpolatedStringExpr ise).getAnInsert()

csharp/ql/lib/semmle/code/csharp/dispatch/Dispatch.qll

@@ -857,7 +857,7 @@ private module Internal {
     private predicate hasDynamicArg(int i, Type argumentType) {
       exists(Expr argument |
         argument = this.getArgument(i) and
-        argument.stripImplicitCasts().getType() instanceof DynamicType and
+        argument.stripImplicit().getType() instanceof DynamicType and
         argumentType = getAPossibleType(argument, _)
       )
     }

csharp/ql/lib/semmle/code/csharp/exprs/Call.qll

@@ -281,6 +281,10 @@ class MethodCall extends Call, QualifiableExpr, LateBindableExpr, @method_invoca
       result = this.getArgument(i - 1)
     else result = this.getArgument(i)
   }
+
+  override Expr stripImplicit() {
+    if this.isImplicit() then result = this.getQualifier().stripImplicit() else result = this
+  }
 }
 
 /**

csharp/ql/lib/semmle/code/csharp/exprs/Expr.qll

@@ -82,10 +82,18 @@ class Expr extends ControlFlowElement, @expr {
   Expr stripCasts() { result = this }
 
   /**
+   * DEPRECATED: Use `stripImplicit` instead.
+   *
    * Gets an expression that is the result of stripping (recursively) all
    * implicit casts from this expression, if any.
    */
-  Expr stripImplicitCasts() { result = this }
+  deprecated Expr stripImplicitCasts() { result = this.stripImplicit() }
+
+  /**
+   * Gets an expression that is the result of stripping (recursively) all
+   * implicit casts and implicit ToString calls from this expression, if any.
+   */
+  Expr stripImplicit() { result = this }
 
   /**
    * Gets the explicit parameter name used to pass this expression as an
@@ -714,8 +722,8 @@ class Cast extends Expr {
 
   override Expr stripCasts() { result = this.getExpr().stripCasts() }
 
-  override Expr stripImplicitCasts() {
-    if this.isImplicit() then result = this.getExpr().stripImplicitCasts() else result = this
+  override Expr stripImplicit() {
+    if this.isImplicit() then result = this.getExpr().stripImplicit() else result = this
   }
 }
 

csharp/ql/src/Likely Bugs/Dynamic/BadDynamicCall.ql

@@ -44,11 +44,11 @@ abstract class BadDynamicCall extends DynamicExpr {
       ultimateSsaDef = ssaDef.getAnUltimateDefinition()
     |
       ultimateSsaDef.getADefinition() =
-        any(AssignableDefinition def | source = def.getSource().stripImplicitCasts())
+        any(AssignableDefinition def | source = def.getSource().stripImplicit())
       or
       ultimateSsaDef.getADefinition() =
         any(AssignableDefinitions::ImplicitParameterDefinition p |
-          source = p.getParameter().getAnAssignedValue().stripImplicitCasts()
+          source = p.getParameter().getAnAssignedValue().stripImplicit()
         )
     )
   }

csharp/ql/src/Likely Bugs/ObjectComparison.ql

@@ -28,7 +28,7 @@ class ReferenceEqualityTestOnObject extends EqualityOperation {
     exists(getObjectOperand(this)) and
     // Neither operand is 'null'.
     not this.getAnOperand() instanceof NullLiteral and
-    not exists(Type t | t = this.getAnOperand().stripImplicitCasts().getType() |
+    not exists(Type t | t = this.getAnOperand().stripImplicit().getType() |
       t instanceof NullType or
       t instanceof ValueType
     ) and

csharp/ql/test/library-tests/dataflow/implicittostring/PrintAst.expected

@@ -0,0 +1,94 @@
+implicitToString.cs:
+#    3| [Class] TestClass
+#    5|   5: [Class] MyClass
+#    5|     4: [InstanceConstructor,PrimaryConstructor] MyClass
+#    7|     5: [Method] ToString
+#    7|       -1: [TypeMention] string
+#    8|       4: [BlockStmt] {...}
+#    9|         0: [ReturnStmt] return ...;
+#    9|           0: [StringLiteralUtf16] "tainted"
+#   13|   6: [Method] Sink
+#   13|     -1: [TypeMention] Void
+#-----|     2: (Parameters)
+#   13|       0: [Parameter] o
+#   13|         -1: [TypeMention] object
+#   13|     4: [BlockStmt] {...}
+#   15|   7: [Method] M1
+#   15|     -1: [TypeMention] Void
+#   16|     4: [BlockStmt] {...}
+#   17|       0: [LocalVariableDeclStmt] ... ...;
+#   17|         0: [LocalVariableDeclAndInitExpr] MyClass x1 = ...
+#   17|           -1: [TypeMention] MyClass
+#   17|           0: [LocalVariableAccess] access to local variable x1
+#   17|           1: [ObjectCreation] object creation of type MyClass
+#   17|             0: [TypeMention] MyClass
+#   18|       1: [LocalVariableDeclStmt] ... ...;
+#   18|         0: [LocalVariableDeclAndInitExpr] String x2 = ...
+#   18|           -1: [TypeMention] string
+#   18|           0: [LocalVariableAccess] access to local variable x2
+#   18|           1: [AddExpr] ... + ...
+#   18|             0: [StringLiteralUtf16] "Hello"
+#   18|             1: [MethodCall] call to method ToString
+#   18|               -1: [LocalVariableAccess] access to local variable x1
+#   19|       2: [ExprStmt] ...;
+#   19|         0: [MethodCall] call to method Sink
+#   19|           0: [LocalVariableAccess] access to local variable x2
+#   22|   8: [Method] M2
+#   22|     -1: [TypeMention] Void
+#   23|     4: [BlockStmt] {...}
+#   24|       0: [LocalVariableDeclStmt] ... ...;
+#   24|         0: [LocalVariableDeclAndInitExpr] MyClass x1 = ...
+#   24|           -1: [TypeMention] MyClass
+#   24|           0: [LocalVariableAccess] access to local variable x1
+#   24|           1: [ObjectCreation] object creation of type MyClass
+#   24|             0: [TypeMention] MyClass
+#   25|       1: [LocalVariableDeclStmt] ... ...;
+#   25|         0: [LocalVariableDeclAndInitExpr] String x2 = ...
+#   25|           -1: [TypeMention] string
+#   25|           0: [LocalVariableAccess] access to local variable x2
+#   25|           1: [AddExpr] ... + ...
+#   25|             0: [StringLiteralUtf16] "Hello"
+#   25|             1: [MethodCall] call to method ToString
+#   25|               -1: [LocalVariableAccess] access to local variable x1
+#   26|       2: [ExprStmt] ...;
+#   26|         0: [MethodCall] call to method Sink
+#   26|           0: [LocalVariableAccess] access to local variable x2
+#   29|   9: [Method] M3
+#   29|     -1: [TypeMention] Void
+#   30|     4: [BlockStmt] {...}
+#   31|       0: [LocalVariableDeclStmt] ... ...;
+#   31|         0: [LocalVariableDeclAndInitExpr] MyClass x1 = ...
+#   31|           -1: [TypeMention] MyClass
+#   31|           0: [LocalVariableAccess] access to local variable x1
+#   31|           1: [ObjectCreation] object creation of type MyClass
+#   31|             0: [TypeMention] MyClass
+#   32|       1: [LocalVariableDeclStmt] ... ...;
+#   32|         0: [LocalVariableDeclAndInitExpr] String x2 = ...
+#   32|           -1: [TypeMention] string
+#   32|           0: [LocalVariableAccess] access to local variable x2
+#   32|           1: [InterpolatedStringExpr] $"..."
+#   32|             0: [StringLiteralUtf16] "Hello "
+#   32|             1: [MethodCall] call to method ToString
+#   32|               -1: [LocalVariableAccess] access to local variable x1
+#   33|       2: [ExprStmt] ...;
+#   33|         0: [MethodCall] call to method Sink
+#   33|           0: [LocalVariableAccess] access to local variable x2
+#   36|   10: [Method] M4
+#   36|     -1: [TypeMention] Void
+#   37|     4: [BlockStmt] {...}
+#   38|       0: [LocalVariableDeclStmt] ... ...;
+#   38|         0: [LocalVariableDeclAndInitExpr] MyClass x1 = ...
+#   38|           -1: [TypeMention] MyClass
+#   38|           0: [LocalVariableAccess] access to local variable x1
+#   38|           1: [ObjectCreation] object creation of type MyClass
+#   38|             0: [TypeMention] MyClass
+#   39|       1: [LocalVariableDeclStmt] ... ...;
+#   39|         0: [LocalVariableDeclAndInitExpr] String x2 = ...
+#   39|           -1: [TypeMention] string
+#   39|           0: [LocalVariableAccess] access to local variable x2
+#   39|           1: [InterpolatedStringExpr] $"..."
+#   39|             0: [StringLiteralUtf16] "Hello "
+#   39|             1: [LocalVariableAccess] access to local variable x1
+#   40|       2: [ExprStmt] ...;
+#   40|         0: [MethodCall] call to method Sink
+#   40|           0: [LocalVariableAccess] access to local variable x2

csharp/ql/test/library-tests/dataflow/implicittostring/PrintAst.qlref

@@ -0,0 +1 @@
+shared/PrintAst.ql