Merge pull request #2944 from dotpaul/tests

PropertySetAnalysis: Handling assignments to flow captures when tracking properties

Author: dotpaul

src/Microsoft.NetCore.Analyzers/UnitTests/Security/DoNotUseInsecureDeserializerBinaryFormatterWithoutBinderTests.cs

@@ -1196,5 +1196,58 @@ public BookRecord DeserializeBookRecord(byte[] bytes)
     }
 }", GetEditorConfigAdditionalFile(editorConfigText), expected);
         }
+
+        [Fact]
+        public void Deserialize_SharedBinderInstance_NoDiagnostic()
+        {
+            VerifyCSharpWithMyBinderDefined(@"
+using System;
+using System.IO;
+using System.Runtime.Serialization;
+using System.Runtime.Serialization.Formatters.Binary;
+using System.Runtime.Remoting.Messaging;
+
+namespace Blah
+{
+    public class Program
+    {
+        public static SerializationBinder B { get; set; }
+
+        private object DoDeserialization(Stream stream)
+        {
+            BinaryFormatter f = new BinaryFormatter();
+            f.Binder = B ?? throw new Exception(""Expected a non-null SerializationBinder"");
+            return f.Deserialize(stream);
+        }
+    }
+}");
+        }
+
+        [Fact]
+        public void Deserialize_SharedBinderInstanceIntermediate_NoDiagnostic()
+        {
+            VerifyCSharpWithMyBinderDefined(@"
+using System;
+using System.IO;
+using System.Runtime.Serialization;
+using System.Runtime.Serialization.Formatters.Binary;
+using System.Runtime.Remoting.Messaging;
+
+namespace Blah
+{
+    public class Program
+    {
+        public static SerializationBinder B { get; set; }
+
+        private object DoDeserialization(Stream stream)
+        {
+            BinaryFormatter f = new BinaryFormatter();
+            SerializationBinder b = B ?? throw new Exception(""Expected a non-null SerializationBinder"");
+            f.Binder = b;
+            return f.Deserialize(stream);
+        }
+    }
+}");
+        }
     }
 }

src/Microsoft.NetCore.Analyzers/UnitTests/Security/UseXmlReaderForSchemaReadTests.cs

@@ -66,6 +66,30 @@ public void TestMethod(XmlReader reader, ValidationEventHandler validationEventH
 }");
         }
 
+        [Fact]
+        public void XmlSchemaReadDocSample1_Solution()
+        {
+            VerifyCSharp(@"
+using System.IO;
+using System.Xml;
+using System.Xml.Schema;
+
+class TestClass
+{
+    public XmlSchema Test
+    {
+        get
+        {
+            var src = """";
+            TextReader tr = new StreamReader(src);
+            XmlReader reader = XmlReader.Create(tr, new XmlReaderSettings() { XmlResolver = null });
+            XmlSchema schema = XmlSchema.Read(reader , null);
+            return schema;
+        }
+    }
+}");
+        }
+
         protected override DiagnosticAnalyzer GetBasicDiagnosticAnalyzer()
         {
             return new UseXmlReaderForSchemaRead();

src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PropertySetAnalysis/PropertySetAnalysis.PropertySetDataFlowOperationVisitor.cs

@@ -241,19 +241,9 @@ protected override PropertySetAbstractValue VisitAssignmentOperation(IAssignment
                     AnalysisEntity targetAnalysisEntity = null;
                     if (operation.Target.Kind == OperationKind.FlowCaptureReference)
                     {
-                        PointsToAbstractValue lValuePointsToAbstractValue = this.GetPointsToAbstractValue(operation.Target);
-                        if (lValuePointsToAbstractValue.LValueCapturedOperations.Count == 1)
+                        if (this.TryUnwrapFlowCaptureReference(operation.Target, out IOperation lValueOperation, OperationKind.PropertyReference, OperationKind.FieldReference))
                         {
-                            IOperation lValueOperation = lValuePointsToAbstractValue.LValueCapturedOperations.First();
-                            if (lValueOperation.Kind == OperationKind.FieldReference
-                                || lValueOperation.Kind == OperationKind.PropertyReference)
-                            {
-                                this.AnalysisEntityFactory.TryCreate(lValueOperation, out targetAnalysisEntity);
-                            }
-                        }
-                        else
-                        {
-                            Debug.Fail("Can LValues FlowCaptureReferences have more than one operation?");
+                            this.AnalysisEntityFactory.TryCreate(lValueOperation, out targetAnalysisEntity);
                         }
                     }
                     else
@@ -295,7 +285,14 @@ protected override PropertySetAbstractValue VisitAssignmentOperation(IAssignment
                     }
                 }
 
-                if (operation.Target is IPropertyReferenceOperation propertyReferenceOperation
+                IPropertyReferenceOperation propertyReferenceOperation = operation.Target as IPropertyReferenceOperation;
+                if (propertyReferenceOperation == null && operation.Target.Kind == OperationKind.FlowCaptureReference)
+                {
+                    this.TryUnwrapFlowCaptureReference(operation.Target, out IOperation lValue, OperationKind.PropertyReference);
+                    propertyReferenceOperation = lValue as IPropertyReferenceOperation;
+                }
+
+                if (propertyReferenceOperation != null
                     && propertyReferenceOperation.Instance != null
                     && this.TrackedTypeSymbols.Any(s => propertyReferenceOperation.Instance.Type.GetBaseTypesAndThis().Contains(s))
                     && this.DataFlowAnalysisContext.PropertyMappers.TryGetPropertyMapper(
@@ -698,6 +695,37 @@ private void MergeInterproceduralResults(IOperation originalOperation)
                     this._visitedLambdas.Add(lambdaOperation);
                 }
             }
+
+            /// <summary>
+            /// Attempts to find the underlying IOperation that a flow capture reference refers to.
+            /// </summary>
+            /// <param name="flowCaptureReferenceOperation">Operation that may be a flow capture reference to look at.</param>
+            /// <param name="unwrappedOperation">The found underlying operation, if any.</param>
+            /// <param name="kinds">Kinds of operations to look for.</param>
+            /// <returns>True if found, false otherwise.</returns>
+            private bool TryUnwrapFlowCaptureReference(IOperation flowCaptureReferenceOperation, out IOperation unwrappedOperation, params OperationKind[] kinds)
+            {
+                unwrappedOperation = null;
+                if (flowCaptureReferenceOperation != null && flowCaptureReferenceOperation.Kind == OperationKind.FlowCaptureReference)
+                {
+                    PointsToAbstractValue lValuePointsToAbstractValue = this.GetPointsToAbstractValue(flowCaptureReferenceOperation);
+                    if (lValuePointsToAbstractValue.LValueCapturedOperations.Count == 1)
+                    {
+                        IOperation lValueOperation = lValuePointsToAbstractValue.LValueCapturedOperations.First();
+                        if (kinds == null || kinds.Contains(lValueOperation.Kind))
+                        {
+                            unwrappedOperation = lValueOperation;
+                            return true;
+                        }
+                    }
+                    else
+                    {
+                        Debug.Fail("Can LValues FlowCaptureReferences have more than one operation?");
+                    }
+                }
+
+                return false;
+            }
         }
     }
 }