[ASM] DecompileDelegate lib bugfix (#4554)

* Initial flawed fix * Working fix * Minor improvement * Linux compilation fix * Added more tests
DataDog · Aug 29, 2023 · 3c8a407 · 3c8a407
1 parent 6e11673
commit 3c8a407
Show file tree

Hide file tree

Showing 9 changed files with 255 additions and 3 deletions.
diff --git a/tracer/src/Datadog.Tracer.Native/iast/dataflow.cpp b/tracer/src/Datadog.Tracer.Native/iast/dataflow.cpp
@@ -78,6 +78,7 @@ static const WSTRING _fixedAssemblyExcludeFilters[] = {
     WStr("vstest.console"),
     WStr("testhost.*"),
     WStr("Oracle.ManagedDataAccess"),
+    WStr("DelegateDecompiler*"),
     LastEntry, // Can't have an empty array. This must be the last element
 };
 static const WSTRING _fixedMethodIncludeFilters[] = {
@@ -105,6 +106,10 @@ static const WSTRING _fixedMethodExcludeFilters[] = {
     WStr("MongoDB.*"),
     LastEntry, // Can't have an empty array. This must be the last element
 };
+static const WSTRING _fixedMethodAttributeExcludeFilters[] = {
+    WStr("DelegateDecompiler.ComputedAttribute"),
+    LastEntry, // Can't have an empty array. This must be the last element
+};
 
 ModuleAspects::ModuleAspects(Dataflow* dataflow, ModuleInfo* module)
 {
@@ -203,6 +208,13 @@ HRESULT Dataflow::Init()
             {
                 _methodExcludeFilters.push_back(_fixedMethodExcludeFilters[x]);
             }
+
+            // Method attribute filters
+            for (int x = 0; _fixedMethodAttributeExcludeFilters[x] != LastEntry; x++)
+            {
+                _methodAttributeExcludeFilters.push_back(_fixedMethodAttributeExcludeFilters[x]);
+            }
+
         //});
     }
     catch (std::exception& err)
@@ -441,10 +453,19 @@ bool Dataflow::IsAssemblyExcluded(const WSTRING& assemblyName, MatchResult* incl
 {
     return IsExcluded(_assemblyIncludeFilters, _assemblyExcludeFilters, assemblyName, includedMatch, excludedMatch);
 }
-bool Dataflow::IsMethodExcluded(const WSTRING& signature, MatchResult* includedMatch, MatchResult* excludedMatch)
+bool Dataflow::IsMethodExcluded(const WSTRING& methodSignature, MatchResult* includedMatch, MatchResult* excludedMatch)
 {
-    return IsExcluded(_methodIncludeFilters, _methodExcludeFilters, signature, includedMatch, excludedMatch);
+    return IsExcluded(_methodIncludeFilters, _methodExcludeFilters, methodSignature, includedMatch, excludedMatch);
 }
+bool Dataflow::IsMethodAttributeExcluded(const WSTRING& attributeName, MatchResult* includedMatch, MatchResult* excludedMatch)
+{
+    return IsExcluded(_methodAttributeIncludeFilters, _methodAttributeExcludeFilters, attributeName, includedMatch, excludedMatch);
+}
+bool Dataflow::HasMethodAttributeExclusions()
+{
+    return _methodAttributeExcludeFilters.size() > 0;
+}
+
 
 ICorProfilerInfo* Dataflow::GetCorProfilerInfo()
 {

diff --git a/tracer/src/Datadog.Tracer.Native/iast/dataflow.h b/tracer/src/Datadog.Tracer.Native/iast/dataflow.h
@@ -63,6 +63,8 @@ namespace iast
         std::vector<WSTRING> _assemblyExcludeFilters;
         std::vector<WSTRING> _methodIncludeFilters;
         std::vector<WSTRING> _methodExcludeFilters;
+        std::vector<WSTRING> _methodAttributeIncludeFilters;
+        std::vector<WSTRING> _methodAttributeExcludeFilters;
 
         bool _traceJitMethods = false;
 
@@ -101,8 +103,11 @@ namespace iast
                                  MatchResult* excludedMatch = nullptr);
         bool IsAssemblyExcluded(const WSTRING& assemblyName, MatchResult* includedMatch = nullptr,
                                 MatchResult* excludedMatch = nullptr);
-        bool IsMethodExcluded(const WSTRING& signature, MatchResult* includedMatch = nullptr,
+        bool IsMethodExcluded(const WSTRING& methodSignature, MatchResult* includedMatch = nullptr,
                               MatchResult* excludedMatch = nullptr);
+        bool IsMethodAttributeExcluded(const WSTRING& attributeName, MatchResult* includedMatch = nullptr,
+                              MatchResult* excludedMatch = nullptr);
+        bool HasMethodAttributeExclusions();
 
         AppDomainInfo* GetAppDomain(AppDomainID id);
         ModuleInfo* GetModuleInfo(ModuleID moduleId);

diff --git a/tracer/src/Datadog.Tracer.Native/iast/method_info.cpp b/tracer/src/Datadog.Tracer.Native/iast/method_info.cpp
@@ -155,6 +155,11 @@ namespace iast
         return ELEMENT_TYPE_VOID;
     }
 
+    std::vector<WSTRING> MemberRefInfo::GetCustomAttributes()
+    {
+        return _module->GetCustomAttributes(_id);
+    }
+
     //----------------------------
 
     FieldInfo::FieldInfo(ModuleInfo* pModuleInfo, mdFieldDef fieldDef) :
@@ -172,6 +177,22 @@ namespace iast
     }
 
 
+    //----------------------------
+
+    PropertyInfo::PropertyInfo(ModuleInfo* pModuleInfo, mdProperty propId) :
+        MemberRefInfo(pModuleInfo, propId)
+    {
+        WCHAR methodName[1024];
+        ULONG methodNameLength;
+        DWORD attrs;
+
+        HRESULT hr = pModuleInfo->_metadataImport->GetPropertyProps(
+            propId, &_typeDef, methodName, 1024, &methodNameLength, &_methodAttributes, &_pSig, &_nSig, nullptr,
+            nullptr, nullptr, &_setter, &_getter, nullptr, 0, nullptr);
+
+        _name = methodName;
+    }
+
     //----------------------------
 
     MethodSpec::MethodSpec(ModuleInfo* pModuleInfo, mdMethodSpec methodSpec) :
@@ -255,6 +276,18 @@ namespace iast
         if (_isExcluded < 0)
         {
             _isExcluded = _module->_dataflow->IsMethodExcluded(GetFullName());
+            if (!_isExcluded && _module->_dataflow->HasMethodAttributeExclusions())
+            {
+                for (auto methodAttribute : GetCustomAttributes())
+                {
+                    if (_module->_dataflow->IsMethodAttributeExcluded(methodAttribute))
+                    {
+                        _isExcluded = true;
+                        break;
+                    }
+                }
+
+            }
         }
         return _isExcluded;
     }
@@ -526,4 +559,30 @@ namespace iast
         _nMethodIL = 0;
         DEL_ARR(_pMethodIL);
     }
+
+    bool MethodInfo::IsPropertyAccessor()
+    {
+        auto name = shared::ToString(_name);
+        return StartsWith(name, "get_") || StartsWith(name, "set_");
+    }
+
+    std::vector<WSTRING> MethodInfo::GetCustomAttributes()
+    {
+        auto res = MemberRefInfo::GetCustomAttributes();
+        if (IsPropertyAccessor())
+        {
+            // Retrieve property Attributes
+            auto properties = _module->GetProperties(_typeDef);
+            for (auto property : properties)
+            {
+                if (property->GetGetterId() != _id && property->GetSetterId() != _id)
+                {
+                    continue;
+                }
+                AddRange(res, _module->GetCustomAttributes(property->GetPropertyId()));
+            }
+        }
+        return res;
+    }
+
 }
diff --git a/tracer/src/Datadog.Tracer.Native/iast/method_info.h b/tracer/src/Datadog.Tracer.Native/iast/method_info.h
@@ -64,6 +64,7 @@ namespace iast
         virtual SignatureInfo* GetSignature();
         ULONG GetParameterCount();
         CorElementType GetReturnCorType();
+        virtual std::vector<WSTRING> GetCustomAttributes();
 
     private:
         std::atomic<unsigned char> _fullNameCounterLock;
@@ -95,6 +96,21 @@ namespace iast
         mdFieldDef GetFieldDef();
     };
 
+    class PropertyInfo : public MemberRefInfo
+    {
+        friend class ILRewriter;
+        friend class ModuleInfo;
+    protected:
+        mdMethodDef _getter;
+        mdMethodDef _setter;
+    public:
+        PropertyInfo(ModuleInfo* pModuleInfo, mdProperty mdProperty);
+
+        inline mdProperty GetPropertyId() { return _id; }
+        inline mdMethodDef GetGetterId() { return _getter; }
+        inline mdMethodDef GetSetterId() { return _setter; }
+    };
+
     class MethodInfo : public MemberRefInfo
     {
         friend class ILRewriter;
@@ -147,5 +163,8 @@ namespace iast
         void ReJITCompilationFinished();
 
         void DumpIL(std::string message = "", ULONG pnMethodIL = 0, LPCBYTE pMethodIL = nullptr);
+
+        bool IsPropertyAccessor();
+        std::vector<WSTRING> GetCustomAttributes() override;
     };
 }
diff --git a/tracer/src/Datadog.Tracer.Native/iast/module_info.cpp b/tracer/src/Datadog.Tracer.Native/iast/module_info.cpp
@@ -40,6 +40,7 @@ ModuleInfo::~ModuleInfo()
     DEL_MAP_VALUES(_specs);
     DEL_MAP_VALUES(_methods);
     DEL_MAP_VALUES(_fields);
+    DEL_MAP_VALUES(_properties);
     DEL_MAP_VALUES(_signatures);
 }
 
@@ -159,6 +160,10 @@ MemberRefInfo* ModuleInfo::GetMemberRefInfo(mdMemberRef token)
     {
         return GetFieldInfo(token);
     }
+    else if (typeFromToken == mdtProperty)
+    {
+        return GetPropertyInfo(token);
+    }
     else if (typeFromToken == mdtMethodSpec)
     {
         return GetMethodSpec(token);
@@ -182,6 +187,12 @@ FieldInfo* ModuleInfo::GetFieldInfo(mdFieldDef fieldDef)
     return Get<mdFieldDef, FieldInfo>(_fields, fieldDef, [this, fieldDef]() { return new iast::FieldInfo(this, fieldDef); });
 }
 
+PropertyInfo* ModuleInfo::GetPropertyInfo(mdProperty propId)
+{
+    CSGUARD(_cs);
+    return Get<mdProperty, PropertyInfo>(_properties, propId, [this, propId]() { return new iast::PropertyInfo(this, propId); });
+}
+
 MethodSpec* ModuleInfo::GetMethodSpec(mdMethodSpec methodSpec)
 {
     CSGUARD(_cs);
@@ -260,6 +271,34 @@ std::vector<MethodInfo*> ModuleInfo::GetMethods(mdTypeDef typeDef)
     this->_metadataImport->CloseEnum(hCorEnum);
     return methods;
 }
+
+std::vector<PropertyInfo*> ModuleInfo::GetProperties(mdTypeDef typeDef)
+{
+    HCORENUM hCorEnum = nullptr;
+    std::vector<PropertyInfo*> res;
+    mdProperty elements[64];
+    ULONG elementCount;
+
+    HRESULT hr = this->_metadataImport->EnumProperties(&hCorEnum, typeDef, elements,
+                                                            sizeof(elements) / sizeof(elements[0]),
+                                                            &elementCount);
+    if (SUCCEEDED(hr) && elementCount > 0)
+    {
+        res.reserve(elementCount);
+        for (ULONG i = 0; i < elementCount; i++)
+        {
+            auto element = GetPropertyInfo(elements[i]);
+            if (element)
+            {
+                res.push_back(element);
+            }
+        }
+    }
+
+    this->_metadataImport->CloseEnum(hCorEnum);
+    return res;
+}
+
 std::vector<MethodInfo*> ModuleInfo::GetMethods(mdTypeDef typeDef, const WSTRING& name)
 {
     HCORENUM hCorEnum = nullptr;
@@ -835,4 +874,39 @@ mdToken ModuleInfo::DefineMemberRef(const WSTRING& moduleName, const WSTRING& ty
         return 0;
     }
 }
+
+std::vector<WSTRING> ModuleInfo::GetCustomAttributes(mdToken token)
+{
+    std::vector<WSTRING> res;
+
+    HCORENUM hCorEnum = nullptr;
+    mdCustomAttribute enumeratedElements[64];
+    ULONG enumCount;
+
+    while (_metadataImport->EnumCustomAttributes(&hCorEnum, token, mdTokenNil, enumeratedElements,
+                                                sizeof(enumeratedElements) / sizeof(enumeratedElements[0]),
+                                                &enumCount) == S_OK)
+    {
+        mdToken attributeParentToken = mdTokenNil;
+        mdToken attributeCtorToken = mdTokenNil;
+        const void* attribute_data = nullptr; // Pointer to receive attribute data, which is not needed for our purposes
+        DWORD data_size = 0;
+
+        for (ULONG i = 0; i < enumCount; i++)
+        {
+            HRESULT hr = _metadataImport->GetCustomAttributeProps(enumeratedElements[i], &attributeParentToken,
+                                                                 &attributeCtorToken, &attribute_data, &data_size);
+            if (SUCCEEDED(hr))
+            {
+                auto attrCtor = GetMemberRefInfo(attributeCtorToken);
+                res.push_back(attrCtor->GetTypeName());
+            }
+        }
+    }
+
+    _metadataImport->CloseEnum(hCorEnum);
+    return res;
+}
+
+
 } // namespace iast
diff --git a/tracer/src/Datadog.Tracer.Native/iast/module_info.h b/tracer/src/Datadog.Tracer.Native/iast/module_info.h
@@ -17,6 +17,7 @@ namespace iast
     class MethodSpec;
     class MethodInfo;
     class FieldInfo;
+    class PropertyInfo;
     class DataflowAspect;
     class DataflowAspectReference;
     class AspectFilter;
@@ -30,10 +31,12 @@ namespace iast
         friend class Dataflow;
         friend class ILRewriter;
         friend class TypeInfo;
+        friend class MemberInfo;
         friend class MemberRefInfo;
         friend class MethodSpec;
         friend class MethodInfo;
         friend class FieldInfo;
+        friend class PropertyInfo;
         friend class DataflowAspectClass;
         friend class DataflowAspect;
         friend class DataflowAspectReference;
@@ -51,6 +54,7 @@ namespace iast
         std::unordered_map<mdMemberRef, MemberRefInfo*> _members;
         std::unordered_map<mdMethodDef, MethodInfo*> _methods;
         std::unordered_map<mdMethodDef, FieldInfo*> _fields;
+        std::unordered_map<mdProperty, PropertyInfo*> _properties;
         std::unordered_map<mdMethodSpec, MethodSpec*> _specs;
         std::unordered_map<mdSignature, SignatureInfo*> _signatures;
 
@@ -117,6 +121,7 @@ namespace iast
         MemberRefInfo* GetMemberRefInfo(mdMemberRef token);
         MethodInfo* GetMethodInfo(mdMethodDef methodDef);
         FieldInfo* GetFieldInfo(mdFieldDef fieldDef);
+        PropertyInfo* GetPropertyInfo(mdProperty propId);
         MethodSpec* GetMethodSpec(mdMethodSpec methodSpec);
         SignatureInfo* GetSignature(mdSignature sigToken);
         WSTRING GetUserString(mdString token);
@@ -126,6 +131,8 @@ namespace iast
         std::vector<MethodInfo*> GetMethods(mdTypeDef typeDef, const WSTRING& methodName, ULONG paramCount);
         std::vector<MethodInfo*> GetMethods(const WSTRING& typeName, const WSTRING& methodName);
 
+        std::vector<PropertyInfo*> GetProperties(mdTypeDef typeDef);
+
         MethodInfo* GetMethod(const WSTRING& typeName, const WSTRING& methodName, PCCOR_SIGNATURE pSignature, ULONG nSignature);
         MethodInfo* GetMethod(mdTypeDef typeDef, const WSTRING& methodName, PCCOR_SIGNATURE pSignature, ULONG nSignature);
         MethodInfo* GetMethod(const WSTRING& typeName, const WSTRING& methodName, int paramCount = 0);
@@ -142,5 +149,7 @@ namespace iast
 
         HRESULT FindMemberRefsByName(mdTypeRef typeRef, const WSTRING& memberName, std::vector<mdMemberRef>& members);
         HRESULT GetAssemblyTypeRef(const WSTRING& assemblyName, const WSTRING& typeName, mdTypeRef* typeRef);
+
+        std::vector<WSTRING> GetCustomAttributes(mdToken token);
     };
 }
diff --git a/...test-applications/integrations/Samples.InstrumentedTests/Samples.InstrumentedTests.csproj b/...test-applications/integrations/Samples.InstrumentedTests/Samples.InstrumentedTests.csproj
@@ -25,6 +25,7 @@
   </PropertyGroup>
 
   <ItemGroup>
+    <PackageReference Include="DelegateDecompiler" Version="0.32.0" />
     <PackageReference Include="FluentAssertions" Version="6.4.0" />
     <PackageReference Include="Moq" Version="4.16.0" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="$(MicrosoftTestSDKVersion)" />

diff --git a/...tions/Samples.InstrumentedTests/Vulnerabilities/SqlInjection/DabaseHelpers/TestClasses.cs b/...tions/Samples.InstrumentedTests/Vulnerabilities/SqlInjection/DabaseHelpers/TestClasses.cs
@@ -5,4 +5,22 @@ public class Book
     public string Id { get; set; }
     public string Title { get; set; }
     public string Author { get; set; }
+
+    [DelegateDecompiler.Computed]
+    public string FullId
+    {
+        get => Id + "-" + Title + "-" + Author;
+    }
+
+    public string FullTitle
+    {
+        [DelegateDecompiler.Computed]
+        get => Title + "_";
+    }
+
+    public string FullAuthor
+    {
+        get => "_" + Author;
+    }
+
 }