Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ASM] DecompileDelegate lib bugfix #4554

Merged
merged 6 commits into from
Aug 29, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions tracer/src/Datadog.Tracer.Native/iast/dataflow.cpp
Original file line number Diff line number Diff line change
@@ -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()
{
7 changes: 6 additions & 1 deletion tracer/src/Datadog.Tracer.Native/iast/dataflow.h
Original file line number Diff line number Diff line change
@@ -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);
59 changes: 59 additions & 0 deletions tracer/src/Datadog.Tracer.Native/iast/method_info.cpp
Original file line number Diff line number Diff line change
@@ -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()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the computed attribute be used in methods other than property accessors?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so. And the getter is a method anyways...

{
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;
}

}
19 changes: 19 additions & 0 deletions tracer/src/Datadog.Tracer.Native/iast/method_info.h
Original file line number Diff line number Diff line change
@@ -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;
};
}
74 changes: 74 additions & 0 deletions tracer/src/Datadog.Tracer.Native/iast/module_info.cpp
Original file line number Diff line number Diff line change
@@ -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
9 changes: 9 additions & 0 deletions tracer/src/Datadog.Tracer.Native/iast/module_info.h
Original file line number Diff line number Diff line change
@@ -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);
};
}
Original file line number Diff line number Diff line change
@@ -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)" />
Original file line number Diff line number Diff line change
@@ -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;
}

}
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
#if !NETCOREAPP2_1
using System;
using System.Data;
using System.Data.Entity.Core.EntityClient;
using System.Data.Entity.Infrastructure;
using System.Data.SqlClient;
using System.Linq;
using DelegateDecompiler;
using FluentAssertions;
using Xunit;

@@ -51,5 +53,49 @@ protected override EntityCommand GetEntityCommand(string title)
var query = new EntityCommand(queryString, conn);
return query;
}

[Fact]
public void TestDelegateDecompileLibBug_PropertyAttribute()
{
var all = (db as ApplicationDbContext).Books.ToList();
all.Count.Should().Be(2);
var book = all[1];
var txt = book.FullId;
var res = (db as ApplicationDbContext).Books.Decompile().Where(x => x.FullId == txt).ToList();
res.Count().Should().Be(1);
}

[Fact]
public void TestDelegateDecompileLibBug_PropertyGetterAttribute()
{
// DecompileDelegate does not support the use of the attribute only on the getter
Assert.Throws<NotSupportedException>(() =>
{
var all = (db as ApplicationDbContext).Books.ToList();
all.Count.Should().Be(2);
var book = all[1];
var txt = book.FullTitle;
var res = (db as ApplicationDbContext).Books.Decompile().Where(x => x.FullTitle == txt).ToList();
res.Count().Should().Be(1);
});
}


[Fact]
public void TestDelegateDecompileLibBug_NoAttribute()
{
// We don't support this scenario
Assert.Throws<NotSupportedException>(() =>
{
var all = (db as ApplicationDbContext).Books.ToList();
all.Count.Should().Be(2);
var book = all[1];
var txt = book.FullAuthor;
var res = (db as ApplicationDbContext).Books.Decompile().Where(x => x.FullAuthor == txt).ToList();
res.Count().Should().Be(1);
});
}


}
#endif