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.

Sign up for GitHub

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

[ASM] DecompileDelegate lib bugfix #4554

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2bf4947
Initial flawed fix
daniel-romano-DD Aug 25, 2023
da7dfc6
Working fix
daniel-romano-DD Aug 25, 2023
64347f5
Reverted unwanted change
daniel-romano-DD Aug 25, 2023
206753e
Minor improvement
daniel-romano-DD Aug 28, 2023
ba0bc83
Linux compilation fix
daniel-romano-DD Aug 28, 2023
8a6bab0
Added more tests
daniel-romano-DD Aug 29, 2023
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = {
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
{
Expand Down
7 changes: 6 additions & 1 deletion tracer/src/Datadog.Tracer.Native/iast/dataflow.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down
59 changes: 59 additions & 0 deletions tracer/src/Datadog.Tracer.Native/iast/method_info.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) :
Expand All @@ -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) :
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ namespace iast
virtual SignatureInfo* GetSignature();
ULONG GetParameterCount();
CorElementType GetReturnCorType();
virtual std::vector<WSTRING> GetCustomAttributes();

private:
std::atomic<unsigned char> _fullNameCounterLock;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ namespace iast
class MethodSpec;
class MethodInfo;
class FieldInfo;
class PropertyInfo;
class DataflowAspect;
class DataflowAspectReference;
class AspectFilter;
Expand All @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
};
}
1 change: 1 addition & 0 deletions ...test-applications/integrations/Samples.InstrumentedTests/Samples.InstrumentedTests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)" />
Expand Down
7 changes: 7 additions & 0 deletions ...tions/Samples.InstrumentedTests/Vulnerabilities/SqlInjection/DabaseHelpers/TestClasses.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,11 @@ public class Book
public string Id { get; set; }
public string Title { get; set; }
public string Author { get; set; }

[DelegateDecompiler.Computed]
public string FullTitle
{
// [DelegateDecompiler.Computed]
Copy link
Member

Choose a reason for hiding this comment

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

Can the attribute be applied here? If so I would add another test.

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've added a test, but DecompileDelegate does not support it

get => Title + "_";
}
}
Loading