Ldlen translation expansion (#203)

* partial progress

* progress

* progress

* progress

* fix

* add another example

* further progress

* formatting fix

* fixes

* undo removal of comment

* example null deref

* comment edit

* progress

* progress

* fix

* add ldlen test

* add unit test for this

* minor edits

* fix test

* edit test

* fix

* bug fix

* updated expected counts

* minor bug fix

* bug fix

* bug fix

* fix bug

* undo debug stuff

* typo

* fix

* remove stash markers

---------

Co-authored-by: Xiaoyu Liu <lixiaoyu@microsoft.com>

Cilsil.Test/Assets/TestClass.cs

@@ -303,6 +303,23 @@ namespace Cilsil.Test.Assets
         {
             result = 0;
         }
+   
+        /// <summary>
+        /// Conditionally creates a null dereference; this method content covers the assignment and
+        /// retrieval of array length. 
+        /// </summary>
+        /// <param name="nullDeref">If <c>true</c>, the null dereference occurs; otherwise, it 
+        /// doesn't.</param>
+        public static void ArrayConditionalNullDeref(bool nullDeref)
+        {
+            var length = nullDeref ? 6 : 5;
+            object nullObj = null;
+            String[] paths = new String[5];
+            if (paths.Length < length)
+            {
+                nullObj.GetHashCode();
+            }
+        }
 
         // This method is used to identify a case in which we were creating a false positive null
         // dereference, as a result of passing a reference to result to AssignZeroByReference

Cilsil.Test/Assets/Utils.cs

@@ -67,6 +67,7 @@ namespace Cilsil.Test.Assets
         public enum TestClassMethod
         {
             None,
+            ArrayConditionalNullDeref,
             CleanupStreamReaderObjectField,
             Cast,
             ExpectNonNullParam,
@@ -489,6 +490,13 @@ namespace Cilsil.Test.Assets
                         throw new ArgumentException("TestCastClass requires 1 argument.");
                     }
                     return GetMethodCall(true);
+                case TestClassMethod.ArrayConditionalNullDeref:
+                    if (args == null || args.Length != 1)
+                    {
+                        throw new ArgumentException(
+                            "ArrayConditionalNullDeref requires 1 argument.");
+                    }
+                    return GetMethodCall(true);
                 case TestClassMethod.NestedExceptionConditionalNullDeref:
                     if (args == null || args.Length != 1)
                     {

Cilsil.Test/E2E/NPETest.cs

@@ -845,5 +845,27 @@ namespace Cilsil.Test.E2E
                                                   .ToLower()
                                    }), GetString(expectedError));
         }
+
+        /// <summary>
+        /// Validates translation of ldlen. NOTE: Pulse seems not to yet interpret array length and
+        /// thus an error occurs in both cases. However, biabduction does.
+        /// </summary>
+        /// <param name="doNullDeref">If <c>true</c>, invoked method creates the null 
+        /// deref. Else, does not.</param>
+        /// <param name="expectedError">The expected error.</param>
+        [DataRow(false, InferError.None)]
+        [DataRow(true, InferError.NULL_DEREFERENCE)]
+        [DataTestMethod]
+        public void NullExceptionTestLdlen(bool doNullDeref, InferError expectedError)
+        {
+            TestRunManager.Run(CallTestClassMethod(
+                                   TestClassMethod.ArrayConditionalNullDeref,
+                                   true,
+                                   args: new string[]
+                                   {
+                                       doNullDeref.ToString()
+                                                  .ToLower()
+                                   }), GetString(expectedError));
+        }
     }
 }

Cilsil/Cil/Parsers/LdelemParser.cs

@@ -14,8 +14,7 @@ namespace Cilsil.Cil.Parsers
         protected override bool ParseCilInstructionInternal(Instruction instruction,
                                                             ProgramState state)
         {
-            Tptr arrayTypeWithPtr;
-            Tarray arrayTypeNoPtr;
+
             switch (instruction.OpCode.Code)
             {
                 case Code.Ldelem_Any:
@@ -30,29 +29,16 @@ namespace Cilsil.Cil.Parsers
                     (var arrayIndex, _) = state.Pop();
                     (var array, var type) = state.Pop();
 
-                    if (!(array is VarExpression arrayVar) || !(type.StripPointer() is Tarray))
-                    {
-                        Log.WriteParserWarning(array, instruction, state);
-                        return false;
-                    }
-                    // Type is either Tarray or a Tptr with a Tarray type underlying it.
-                    if (type is Tarray)
-                    {
-                        arrayTypeWithPtr = new Tptr(Tptr.PtrKind.Pk_pointer, type);
-                        arrayTypeNoPtr = (Tarray)type;
-                    }
-                    else if (type is Tptr)
-                    {
-                        arrayTypeWithPtr = (Tptr)type;
-                        arrayTypeNoPtr = (Tarray)type.StripPointer();
-                    }
-                    else
+                    var typeNoPointer = type.StripPointer();
+
+                    if (!(array is VarExpression arrayVar) || 
+                        !(typeNoPointer is Tarray arrayTypeNoPtr))
                     {
                         Log.WriteParserWarning(type, instruction, state);
                         return false;
                     }
+                    var derefArray = CreateDereference(arrayVar, type, state);
 
-                    var derefArray = CreateDereference(arrayVar, arrayTypeWithPtr, state);
                     var tempIdentifier = state.GetIdentifier(Identifier.IdentKind.Normal);
                     var arrayIndexLoad = new Load(identifierAssignedTo: tempIdentifier,
                                                   lvalue: new LindexExpression(

Cilsil/Cil/Parsers/LdlenParser.cs

@@ -0,0 +1,57 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+using Cilsil.Sil;
+using Cilsil.Sil.Expressions;
+using Cilsil.Sil.Instructions;
+using Cilsil.Sil.Types;
+using Cilsil.Utils;
+using Mono.Cecil.Cil;
+using System.Collections.Generic;
+
+namespace Cilsil.Cil.Parsers
+{
+    internal class LdlenParser : InstructionParser
+    {
+        protected override bool ParseCilInstructionInternal(Instruction instruction,
+                                                            ProgramState state)
+        {
+            switch (instruction.OpCode.Code)
+            {
+                case Code.Ldlen:
+                    (var array, var type) = state.Pop();
+
+                    var typeNoPointer = type.StripPointer();
+
+                    if (!(array is VarExpression arrayVar) ||
+                        !(typeNoPointer is Tarray))
+                    {
+                        Log.WriteParserWarning(type, instruction, state);
+                        return false;
+                    }
+                    var derefArray = CreateDereference(arrayVar, type, state);
+                    var tempIdentifier = state.GetIdentifier(Identifier.IdentKind.Normal);
+                    var arrayLengthType = new Tint(Tint.IntKind.IInt);
+                    var arrayLengthCall = new Call(tempIdentifier,
+                                                   arrayLengthType,
+                                                   new ConstExpression(
+                                                       ProcedureName.BuiltIn__get_array_length),
+                                                   new List<Call.CallArg> 
+                                                   { 
+                                                       new Call.CallArg(array, type) 
+                                                   },
+                                                   new Call.CallFlags(),
+                                                   state.CurrentLocation);
+                    var newNode = AddMethodBodyInstructionsToCfg(state, derefArray, arrayLengthCall);
+                    state.PushExpr(new VarExpression(tempIdentifier),
+                                   arrayLengthType);
+
+                    state.PushInstruction(instruction.Next, newNode);
+                    state.AppendToPreviousNode = true;
+
+                    return true;
+                default:
+                    return false;
+            }
+        }
+    }
+}

Cilsil/Cil/Parsers/LeaveParser.cs

@@ -71,10 +71,10 @@ namespace Cilsil.Cil.Parsers
 
                 case Code.Throw:
                     (var returnValue, _) = state.Pop();
-                    var retNode = CreateExceptionReturnNode(state,
-                                                            returnValue,
-                                                            state.CurrentLocation);
-                    RegisterNode(state, retNode);
+                    var throwNode = CreateExceptionReturnNode(state,
+                                                              returnValue,
+                                                              state.CurrentLocation);
+                    HandleFinallyControlFlowForThrow(state, instruction, throwNode);
                     return true;
                 case Code.Rethrow:
                     var exceptionType = 
@@ -87,9 +87,9 @@ namespace Cilsil.Cil.Parsers
                         exceptionType, state);
                     state.PreviousNode.Instructions.Add(memoryAllocationCall);
                     var rethrowNode = CreateExceptionReturnNode(state,
-                                                                objectVariable,
-                                                                state.CurrentLocation);
-                    RegisterNode(state, rethrowNode);
+                                        objectVariable,
+                                        state.CurrentLocation);
+                    HandleFinallyControlFlowForThrow(state, instruction, rethrowNode);
                     return true;
                 default:
                     return false;
@@ -152,5 +152,29 @@ namespace Cilsil.Cil.Parsers
                 state.PushInstruction(targetInstr);
             }
         }
+
+        private void HandleFinallyControlFlowForThrow(
+            ProgramState state, Instruction instruction, CfgNode throwNode)
+        {
+            // Before control flow leaves this block via the throw, we need to route
+            // control flow through the finally block.
+            if (state.MethodExceptionHandlers
+                     .TryOffsetToFinallyHandler
+                     .ContainsKey(instruction.Offset))
+            {
+                var finallyHandler =
+                    state.MethodExceptionHandlers
+                         .TryOffsetToFinallyHandler[instruction.Offset]
+                         .Item1;
+                state.PushInstruction(finallyHandler.HandlerStart,
+                                      CreateFinallyHandlerNonExceptionalEntry(
+                                          state, finallyHandler, null, throwNode));
+            }
+            else
+            {
+                RegisterNode(state, throwNode);
+            }
+
+        }
     }
 }

Cilsil/Sil/ProcedureName.cs

@@ -107,6 +107,17 @@ namespace Cilsil.Sil
                               BuiltInClassName,
                               string.Empty,
                               true);
+
+        /// <summary>
+        /// Standard procedure name for getting the length of an array, interpreted specially by
+        /// the backend.
+        /// </summary>
+        public static readonly ProcedureName BuiltIn__get_array_length =
+            new ProcedureName("__get_array_length",
+                              new List<string>(),
+                              BuiltInClassName,
+                              string.Empty,
+                              true);
         #endregion
 
         /// <summary>

Examples/proj/Program.cs

@@ -36,7 +36,7 @@ public class IsDisposedBooleanField : IDisposable
     }
 }
 
-// Expect 4 TAINT_ERROR for SQL injection flows.
+// Expect 5 TAINT_ERROR for SQL injection flows.
 public class PulseTaintTests
 {
     [HttpPost]
@@ -88,6 +88,24 @@ public class PulseTaintTests
         }
     }
 
+    [HttpGet]
+    public static void PropagateTaintArrayIterator(string[] roleIds)
+    {
+        foreach (var roleId in roleIds)
+        {
+            using var command = new SqlCommand(roleId);
+        }
+    }
+
+    [HttpGet]
+    public static void PropagateTaintArrayIterateByIndex(string[] roleIds)
+    {
+        for (int i = 0; i < roleIds.Length; i++)
+        {
+            using var command = new SqlCommand(roleIds[i]);
+        }
+    }
+
 }
 
 
@@ -229,7 +247,7 @@ public class MainClass
     }
 }
 
-// 18 reports expected (19 with --pulse-increase-leak-recall flag)
+// 19 reports expected (20 with --pulse-increase-leak-recall flag)
 class InferResourceLeakTests
 {
     private static byte[] myBytes = new byte[] { 10, 4 };