From b0561fe42f5d09eb717b56449c1ff71483800db9 Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Fri, 19 Nov 2021 11:33:04 -0800 Subject: [PATCH 1/3] Add AMSI method invocation logging as experimental feature --- .../ExperimentalFeature.cs | 4 + .../engine/parser/Compiler.cs | 4 +- .../engine/runtime/Binding/Binders.cs | 16 ++ .../engine/runtime/Operations/MiscOps.cs | 65 +++++++ .../security/SecuritySupport.cs | 175 +++++++++++++++--- 5 files changed, 234 insertions(+), 30 deletions(-) diff --git a/src/System.Management.Automation/engine/ExperimentalFeature/ExperimentalFeature.cs b/src/System.Management.Automation/engine/ExperimentalFeature/ExperimentalFeature.cs index 5695d54806d..bfebfd6d2f6 100644 --- a/src/System.Management.Automation/engine/ExperimentalFeature/ExperimentalFeature.cs +++ b/src/System.Management.Automation/engine/ExperimentalFeature/ExperimentalFeature.cs @@ -25,6 +25,7 @@ public class ExperimentalFeature internal const string PSNativeCommandErrorActionPreferenceFeatureName = "PSNativeCommandErrorActionPreference"; internal const string PSRemotingSSHTransportErrorHandling = "PSRemotingSSHTransportErrorHandling"; internal const string PSCleanBlockFeatureName = "PSCleanBlock"; + internal const string PSAMSIMethodInvocationLogging = "PSAMSIMethodInvocationLogging"; #endregion @@ -134,6 +135,9 @@ static ExperimentalFeature() new ExperimentalFeature( name: PSCleanBlockFeatureName, description: "Add support of a 'Clean' block to functions and script cmdlets for easy resource cleanup"), + new ExperimentalFeature( + name: PSAMSIMethodInvocationLogging, + description: "Provides AMSI notification of .NET method invocations."), }; EngineExperimentalFeatures = new ReadOnlyCollection(engineFeatures); diff --git a/src/System.Management.Automation/engine/parser/Compiler.cs b/src/System.Management.Automation/engine/parser/Compiler.cs index 9956fb29e30..b8ca57b4b05 100644 --- a/src/System.Management.Automation/engine/parser/Compiler.cs +++ b/src/System.Management.Automation/engine/parser/Compiler.cs @@ -639,7 +639,9 @@ internal static class CachedReflectionInfo internal static readonly MethodInfo ArgumentTransformationAttribute_Transform = typeof(ArgumentTransformationAttribute).GetMethod(nameof(ArgumentTransformationAttribute.Transform), InstancePublicFlags); - // ReSharper restore InconsistentNaming + + internal static readonly MethodInfo MemberInvocationLoggingOps_LogMemberInvocation = + typeof(MemberInvocationLoggingOps).GetMethod(nameof(MemberInvocationLoggingOps.LogMemberInvocation), StaticFlags); } internal static class ExpressionCache diff --git a/src/System.Management.Automation/engine/runtime/Binding/Binders.cs b/src/System.Management.Automation/engine/runtime/Binding/Binders.cs index a4917d5af6b..7f3832716f0 100644 --- a/src/System.Management.Automation/engine/runtime/Binding/Binders.cs +++ b/src/System.Management.Automation/engine/runtime/Binding/Binders.cs @@ -6903,6 +6903,22 @@ internal static DynamicMetaObject InvokeDotNetMethod( expr = Expression.Block(expr, ExpressionCache.AutomationNullConstant); } + if (ExperimentalFeature.IsEnabled(ExperimentalFeature.PSAMSIMethodInvocationLogging)) + { + // Expression block runs two expressions in order: + // - Log method invocation to AMSI Notifications (can throw PSSecurityException) + // - Invoke method + expr = Expression.Block( + Expression.Call( + CachedReflectionInfo.MemberInvocationLoggingOps_LogMemberInvocation, + target.Expression, + Expression.Constant(name), + Expression.NewArrayInit( + typeof(object), + args.Select(static e => e.Expression.Cast(typeof(object))))), + expr); + } + // If we're calling SteppablePipeline.{Begin|Process|End}, we don't want // to wrap exceptions - this is very much a special case to help error // propagation and ensure errors are attributed to the correct code (the diff --git a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs index 8d50fcf5f1a..9c1ed035dfd 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs @@ -3546,4 +3546,69 @@ internal static object[] GetSlice(IList list, int startIndex) return result; } } + + internal static class MemberInvocationLoggingOps + { + private static readonly Lazy DumpLogAMSIContent = new Lazy( + () => { + object result = Environment.GetEnvironmentVariable("__PSDumpAMSILogContent"); + if (result != null && LanguagePrimitives.TryConvertTo(result, out int value)) + { + return value == 1; + } + return false; + } + ); + + internal static void LogMemberInvocation(object target, string name, object[] args) + { + try + { + var contentName = "PowerShellMemberInvocation"; + var argsBuilder = new Text.StringBuilder(); + + for (int i = 0; i < args.Length; i++) + { + string value = args[i] is null ? "null" : args[i].ToString(); + + if (i > 0) + { + argsBuilder.Append(", "); + } + + argsBuilder.Append($"<{value}>"); + } + + string content = $"<{target}>.{name}({argsBuilder})"; + + if (DumpLogAMSIContent.Value) + { + Console.WriteLine("\n=== Amsi notification report content ==="); + Console.WriteLine(content); + } + + var success = AmsiUtils.ReportContent( + name: contentName, + content: content); + + if (DumpLogAMSIContent.Value) + { + Console.WriteLine($"=== Amsi notification report success: {success} ==="); + } + } + catch (PSSecurityException) + { + // ReportContent() will throw PSSecurityException if AMSI detects malware, which + // must be propagated. + throw; + } + catch (Exception ex) + { + if (DumpLogAMSIContent.Value) + { + Console.WriteLine($"!!! Amsi notification report exception: {ex} !!!"); + } + } + } + } } diff --git a/src/System.Management.Automation/security/SecuritySupport.cs b/src/System.Management.Automation/security/SecuritySupport.cs index e41b64db625..7b5087411ec 100644 --- a/src/System.Management.Automation/security/SecuritySupport.cs +++ b/src/System.Management.Automation/security/SecuritySupport.cs @@ -1383,7 +1383,10 @@ internal static AmsiNativeMethods.AMSI_RESULT ScanContent(string content, string #endif } - internal static AmsiNativeMethods.AMSI_RESULT WinScanContent(string content, string sourceMetadata, bool warmUp) + internal static AmsiNativeMethods.AMSI_RESULT WinScanContent( + string content, + string sourceMetadata, + bool warmUp) { if (string.IsNullOrEmpty(sourceMetadata)) { @@ -1414,33 +1417,9 @@ internal static AmsiNativeMethods.AMSI_RESULT WinScanContent(string content, str try { - int hr = 0; - - // Initialize AntiMalware Scan Interface, if not already initialized. - // If we failed to initialize previously, just return the neutral result ("AMSI_RESULT_NOT_DETECTED") - if (s_amsiContext == IntPtr.Zero) + if (!CheckAmsiInit()) { - hr = Init(); - - if (!Utils.Succeeded(hr)) - { - s_amsiInitFailed = true; - return AmsiNativeMethods.AMSI_RESULT.AMSI_RESULT_NOT_DETECTED; - } - } - - // Initialize the session, if one isn't already started. - // If we failed to initialize previously, just return the neutral result ("AMSI_RESULT_NOT_DETECTED") - if (s_amsiSession == IntPtr.Zero) - { - hr = AmsiNativeMethods.AmsiOpenSession(s_amsiContext, ref s_amsiSession); - AmsiInitialized = true; - - if (!Utils.Succeeded(hr)) - { - s_amsiInitFailed = true; - return AmsiNativeMethods.AMSI_RESULT.AMSI_RESULT_NOT_DETECTED; - } + return AmsiNativeMethods.AMSI_RESULT.AMSI_RESULT_NOT_DETECTED; } if (warmUp) @@ -1453,6 +1432,7 @@ internal static AmsiNativeMethods.AMSI_RESULT WinScanContent(string content, str AmsiNativeMethods.AMSI_RESULT result = AmsiNativeMethods.AMSI_RESULT.AMSI_RESULT_CLEAN; // Run AMSI content scan + int hr; unsafe { fixed (char* buffer = content) @@ -1484,6 +1464,123 @@ internal static AmsiNativeMethods.AMSI_RESULT WinScanContent(string content, str } } + /// + /// Reports provided content to AMSI (Antimalware Scan Interface). + /// + /// Name of content being reported. + /// Content being reported. + /// True if content was successfully reported. + internal static bool ReportContent( + string name, + string content) + { +#if UNIX + return false; +#else + return WinReportContent(name, content); +#endif + } + + private static bool WinReportContent( + string name, + string content) + { + if (string.IsNullOrEmpty(name) || + string.IsNullOrEmpty(content) || + s_amsiInitFailed || + s_amsiNotifyFailed) + { + return false; + } + + lock (s_amsiLockObject) + { + if (s_amsiNotifyFailed) + { + return false; + } + + try + { + if (!CheckAmsiInit()) + { + return false; + } + + int hr; + AmsiNativeMethods.AMSI_RESULT result = AmsiNativeMethods.AMSI_RESULT.AMSI_RESULT_NOT_DETECTED; + unsafe + { + fixed (char* buffer = content) + { + var buffPtr = new IntPtr(buffer); + hr = AmsiNativeMethods.AmsiNotifyOperation( + amsiContext: s_amsiContext, + buffer: buffPtr, + length: (uint)(content.Length * sizeof(char)), + contentName: name, + ref result); + } + } + + if (Utils.Succeeded(hr)) + { + if (result == AmsiNativeMethods.AMSI_RESULT.AMSI_RESULT_DETECTED) + { + // If malware is detected, throw to prevent method invoke expression from running. + throw new PSSecurityException(ParserStrings.ScriptContainedMaliciousContent); + } + + return true; + } + + return false; + } + catch (DllNotFoundException) + { + s_amsiNotifyFailed = true; + return false; + } + catch (System.EntryPointNotFoundException) + { + s_amsiNotifyFailed = true; + return false; + } + } + } + + private static bool CheckAmsiInit() + { + // Initialize AntiMalware Scan Interface, if not already initialized. + // If we failed to initialize previously, just return the neutral result ("AMSI_RESULT_NOT_DETECTED") + if (s_amsiContext == IntPtr.Zero) + { + int hr = Init(); + + if (!Utils.Succeeded(hr)) + { + s_amsiInitFailed = true; + return false; + } + } + + // Initialize the session, if one isn't already started. + // If we failed to initialize previously, just return the neutral result ("AMSI_RESULT_NOT_DETECTED") + if (s_amsiSession == IntPtr.Zero) + { + int hr = AmsiNativeMethods.AmsiOpenSession(s_amsiContext, ref s_amsiSession); + AmsiInitialized = true; + + if (!Utils.Succeeded(hr)) + { + s_amsiInitFailed = true; + return false; + } + } + + return true; + } + internal static void CurrentDomain_ProcessExit(object sender, EventArgs e) { if (AmsiInitialized && !AmsiUninitializeCalled) @@ -1499,6 +1596,7 @@ internal static void CurrentDomain_ProcessExit(object sender, EventArgs e) private static IntPtr s_amsiSession = IntPtr.Zero; private static bool s_amsiInitFailed = false; + private static bool s_amsiNotifyFailed = false; private static readonly object s_amsiLockObject = new object(); /// @@ -1623,8 +1721,27 @@ internal static extern int AmsiInitialize( [DefaultDllImportSearchPathsAttribute(DllImportSearchPath.System32)] [DllImportAttribute("amsi.dll", EntryPoint = "AmsiScanBuffer", CallingConvention = CallingConvention.StdCall)] internal static extern int AmsiScanBuffer( - System.IntPtr amsiContext, System.IntPtr buffer, uint length, - [InAttribute()][MarshalAsAttribute(UnmanagedType.LPWStr)] string contentName, System.IntPtr amsiSession, ref AMSI_RESULT result); + System.IntPtr amsiContext, + System.IntPtr buffer, + uint length, + [InAttribute()][MarshalAsAttribute(UnmanagedType.LPWStr)] string contentName, + System.IntPtr amsiSession, + ref AMSI_RESULT result); + + /// Return Type: HRESULT->LONG->int + /// amsiContext: HAMSICONTEXT->HAMSICONTEXT__* + /// buffer: PVOID->void* + /// length: ULONG->unsigned int + /// contentName: LPCWSTR->WCHAR* + /// result: AMSI_RESULT* + [DefaultDllImportSearchPathsAttribute(DllImportSearchPath.System32)] + [DllImportAttribute("amsi.dll", EntryPoint = "AmsiNotifyOperation", CallingConvention = CallingConvention.StdCall)] + internal static extern int AmsiNotifyOperation( + System.IntPtr amsiContext, + System.IntPtr buffer, + uint length, + [InAttribute()][MarshalAsAttribute(UnmanagedType.LPWStr)] string contentName, + ref AMSI_RESULT result); /// Return Type: HRESULT->LONG->int ///amsiContext: HAMSICONTEXT->HAMSICONTEXT__* From 5b0123694990a44071f5cd24360d57ad228afa29 Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Mon, 29 Nov 2021 16:18:59 -0800 Subject: [PATCH 2/3] Add fix for value type errors in logging expression --- .../engine/runtime/Binding/Binders.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/runtime/Binding/Binders.cs b/src/System.Management.Automation/engine/runtime/Binding/Binders.cs index 7f3832716f0..984edfaa040 100644 --- a/src/System.Management.Automation/engine/runtime/Binding/Binders.cs +++ b/src/System.Management.Automation/engine/runtime/Binding/Binders.cs @@ -6911,7 +6911,7 @@ internal static DynamicMetaObject InvokeDotNetMethod( expr = Expression.Block( Expression.Call( CachedReflectionInfo.MemberInvocationLoggingOps_LogMemberInvocation, - target.Expression, + target.Expression.Cast(typeof(object)), Expression.Constant(name), Expression.NewArrayInit( typeof(object), From bb46ce3fc5f95364daf53df213c7e5e971ffb949 Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Tue, 30 Nov 2021 11:30:56 -0800 Subject: [PATCH 3/3] Fix recursion error --- .../engine/runtime/Binding/Binders.cs | 3 ++- .../engine/runtime/Operations/MiscOps.cs | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/System.Management.Automation/engine/runtime/Binding/Binders.cs b/src/System.Management.Automation/engine/runtime/Binding/Binders.cs index 984edfaa040..52f85e372b2 100644 --- a/src/System.Management.Automation/engine/runtime/Binding/Binders.cs +++ b/src/System.Management.Automation/engine/runtime/Binding/Binders.cs @@ -6908,10 +6908,11 @@ internal static DynamicMetaObject InvokeDotNetMethod( // Expression block runs two expressions in order: // - Log method invocation to AMSI Notifications (can throw PSSecurityException) // - Invoke method + string targetName = methodInfo.ReflectedType?.FullName ?? string.Empty; expr = Expression.Block( Expression.Call( CachedReflectionInfo.MemberInvocationLoggingOps_LogMemberInvocation, - target.Expression.Cast(typeof(object)), + Expression.Constant(targetName), Expression.Constant(name), Expression.NewArrayInit( typeof(object), diff --git a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs index 9c1ed035dfd..62468dcfc3f 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs @@ -3560,7 +3560,7 @@ internal static class MemberInvocationLoggingOps } ); - internal static void LogMemberInvocation(object target, string name, object[] args) + internal static void LogMemberInvocation(string targetName, string name, object[] args) { try { @@ -3579,7 +3579,7 @@ internal static void LogMemberInvocation(object target, string name, object[] ar argsBuilder.Append($"<{value}>"); } - string content = $"<{target}>.{name}({argsBuilder})"; + string content = $"<{targetName}>.{name}({argsBuilder})"; if (DumpLogAMSIContent.Value) {