Skip to content

Blacklist System.Windows.Forms form loaded to prevent a crash - #6822

Merged
Travis Plunk (TravisEz13) merged 7 commits into
PowerShell:masterfrom
TravisEz13:blacklist_winforms
May 7, 2018
Merged

Blacklist System.Windows.Forms form loaded to prevent a crash#6822
Travis Plunk (TravisEz13) merged 7 commits into
PowerShell:masterfrom
TravisEz13:blacklist_winforms

Conversation

@TravisEz13

@TravisEz13 Travis Plunk (TravisEz13) commented May 5, 2018

Copy link
Copy Markdown
Member

PR Summary

Blacklist System.Windows.Forms (WinForms) from being loaded to prevent a crash

Before the change

WinForms would load but System.Drawing would not when you actually tried to display a form.
image
Then when the form was disposed the process would crash with a TypeLoadException. Here are details from the crash dump:
image

After this change, we refuse to load WinForms preventing the crash:
image

PR Checklist

$ErrorId
)

{[System.Reflection.Assembly]::Load('system.windows.forms')} | Should -Throw -ErrorId $ErrorId

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Typo - should be Load($Name)

@@ -0,0 +1,33 @@
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License.
Describe "Assembly.Load Validation Test" -Tags "CI" {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add empty line after copyright comment.

$ErrorId
)

{[System.Reflection.Assembly]::Load($Name)} | Should -Throw -ErrorId $ErrorId

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Assembly.Load will not work if the assembly name is not a long form name. LoadWithPartialName should be used here instead. Or the name used should be System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Assembly.Load seems working with short name too:

PS:1> [System.Reflection.Assembly]::Load("System.Management.Automation")

GAC    Version        Location
---    -------        --------
False  v4.0.30319     F:\pscore\System.Management.Automation.dll`

@daxian-dbw

Copy link
Copy Markdown
Member

LGTM, but please address the pending comments.

@TravisEz13 Travis Plunk (TravisEz13) added the WG-Engine core PowerShell engine, interpreter, and runtime label May 7, 2018
@TravisEz13
Travis Plunk (TravisEz13) merged commit 8f37c92 into PowerShell:master May 7, 2018
@TravisEz13
Travis Plunk (TravisEz13) deleted the blacklist_winforms branch May 7, 2018 23:38
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
…hell#6822)

Blacklist `System.Windows.Forms` (`WinForms`) from being loaded to prevent a crash
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WG-Engine core PowerShell engine, interpreter, and runtime

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants