Merge pull request github#18446 from michaelnebel/csharp/implicittostring2

C#: Adding synthetic implicit ToString calls in binary- and string interpolation expressions.

Author: michaelnebel

csharp/extractor/Semmle.Extraction.CSharp/CodeAnalysisExtensions/SymbolExtensions.cs

@@ -532,6 +532,12 @@ attribute.AttributeClass is INamedTypeSymbol nt &&
             return isInline;
         }
 
+        /// <summary>
+        /// Returns true if this type implements `System.IFormattable`.
+        /// </summary>
+        public static bool ImplementsIFormattable(this ITypeSymbol type) =>
+            type.AllInterfaces.Any(i => i.Name == "IFormattable" && i.ContainingNamespace.ToString() == "System");
+
         /// <summary>
         /// Holds if this type is of the form <code>System.ReadOnlySpan&lt;byte&gt;</code>.
         /// </summary>

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, AnnotatedTypeSymbol.CreateNotAnnotated(toString.ReturnType), 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.Type?.Symbol) is IMethodSymbol toString)
+            {
+                return new ImplicitToString(info, toString);
+            }
+            return Factory.Create(info);
+        }
+    }
+}

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

@@ -1,4 +1,5 @@
 using System.IO;
+using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Semmle.Extraction.Kinds;
@@ -20,7 +21,15 @@ protected override void PopulateExpression(TextWriter trapFile)
                 {
                     case SyntaxKind.Interpolation:
                         var interpolation = (InterpolationSyntax)c;
-                        Create(Context, interpolation.Expression, this, child++);
+                        var exp = interpolation.Expression;
+                        if (Context.GetTypeInfo(exp).Type is ITypeSymbol type && !type.ImplementsIFormattable())
+                        {
+                            ImplicitToString.Create(Context, exp, this, child++);
+                        }
+                        else
+                        {
+                            Create(Context, exp, this, child++);
+                        }
                         break;
                     case SyntaxKind.InterpolatedStringText:
                         // Create a string literal

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 and string interpolation expressions.

csharp/ql/lib/ext/Microsoft.AspNetCore.Http.model.yml

@@ -0,0 +1,6 @@
+extensions:
+  - addsTo:
+      pack: codeql/csharp-all
+      extensible: summaryModel
+    data:
+      - ["Microsoft.AspNetCore.Http", "PathString", True, "ToString", "()", "", "Argument[this]", "ReturnValue", "taint", "manual"]