Powershell: AMSI scan should be done outside the static lock

[PaulHigin]

Currently our AMSI code (https://github.com/PowerShell/PowerShell/blob/master/src/System.Management.Automation/security/SecuritySupport.cs#L1672) calls the AmsiScanString() method within a static lock.

This is unnecessary because the AMSI API is thread safe and designed to process multiple scan requests.

But this code also needs to be refactored because it is unnecessarily un-initializing and re-initializing AMSI when multiple runspaces are used. In addition it is not tracking pipeline sessions correctly for multiple runspaces.

So I see the work items as:

1. Ensure AMSI is initialized and un-initialized only once (see related issue #2334).
2. Add concurrent dictionary to correctly track AMSI session per pipeline.
3. Don't call AmsiScanString within the static lock.

[iSazonov]

It seems fixing this could seed up PowerShell so it would be nice to implement this before 7.0 release.

[PaulHigin]

@iSazonov thanks for the reminder, I had forgotten about this. I don't know if I'll have time to make these changes for 7.0, but I'll make an effort.

[iSazonov]

For reference #8713 - it is seem fixed (1).

[iSazonov]

AmsiScanString is never called. I wonder.

[PaulHigin]

I forgot about this issue, but the work still needs to be done. We now call AmsiScanBuffer(), but still do it under a lock. I don't think this is high priority, but would be nice to do at some point.

https://github.com/PowerShell/PowerShell/blob/master/src/System.Management.Automation/security/SecuritySupport.cs#L1422

GitHub

PowerShell/PowerShell

PowerShell for every system! Contribute to PowerShell/PowerShell development by creating an account on GitHub.

[iSazonov]

As side note, the code PsUtils.GetMainModule(currentProcess); could be replaced with Assembly.GetEntryAssembly() for performance.