Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Handle case where applocker test script fails to delete#8627

Merged
TravisEz13 merged 2 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
SteveL-MSFT:applocker-policySteveL-MSFT/PowerShell:applocker-policyCopy head branch name to clipboard
Jan 16, 2019
Merged

Handle case where applocker test script fails to delete#8627
TravisEz13 merged 2 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
SteveL-MSFT:applocker-policySteveL-MSFT/PowerShell:applocker-policyCopy head branch name to clipboard

Conversation

@SteveL-MSFT

@SteveL-MSFT SteveL-MSFT commented Jan 11, 2019

Copy link
Copy Markdown
Member

PR Summary

One reported case where during applocker policy check, the test script that is created cannot be deleted because something else (malware scanner?) has a lock on the file. This results in an exception being thrown in finally block which ultimately causes PowerShell to crash due to the unhandled exception. Fix is to wrap the deletion in the finally block with try..catch and ignore any exceptions and leave the temp file.

TFS:20156282

PR Checklist

Comment thread src/System.Management.Automation/security/wldpNativeMethods.cs Outdated
@iSazonov iSazonov added the CL-Engine Indicates that a PR should be marked as an engine change in the Change Log label Jan 12, 2019
@iSazonov

iSazonov commented Jan 12, 2019

Copy link
Copy Markdown
Collaborator

If we can not delete the file is it secure to leave it on file system?

@SteveL-MSFT

Copy link
Copy Markdown
Member Author

@iSazonov yes, the test script is harmless and doesn't expose anything, it just validates whether scripts can be run or not. It is not ideal to leave the file in the temp folder, but worse is to prevent PowerShell from starting simply because something else decided to lock that file.

@SteveL-MSFT

Copy link
Copy Markdown
Member Author

@PaulHigin can you take another look? Made some changes based on feedback

@iSazonov

Copy link
Copy Markdown
Collaborator

@TravisEz13 Please merge the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-Engine Indicates that a PR should be marked as an engine change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.