Skip to content

[release/v7.7.0-preview.1] Update MaxVisitCount and MaxHashtableKeyCount if VisitorSafeValueContext indicates SkipLimitCheck is true #27308

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

Merged
jshigetomi merged 1 commit into from
Apr 20, 2026
+15 −5
Merged

[release/v7.7.0-preview.1] Update MaxVisitCount and MaxHashtableKeyCount if VisitorSafeValueContext indicates SkipLimitCheck is true #27308

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 9 additions & 5 deletions src/System.Management.Automation/engine/parser/SafeValues.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,15 @@ public static bool IsAstSafe(Ast ast, GetSafeValueVisitor.SafeValueContext safeV
internal IsSafeValueVisitor(GetSafeValueVisitor.SafeValueContext safeValueContext)
{
_safeValueContext = safeValueContext;

bool skipSizeCheck = safeValueContext is GetSafeValueVisitor.SafeValueContext.SkipHashtableSizeCheck;
_maxVisitCount = skipSizeCheck ? uint.MaxValue : 5000;
_maxHashtableKeyCount = skipSizeCheck ? int.MaxValue : 500;
Comment on lines +51 to +53
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

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

The default limits (5000 visits / 500 hashtable keys) are now hard-coded literals in the constructor. Consider keeping these as named constants (e.g., DefaultMaxVisitCount / DefaultMaxHashtableKeyCount) to avoid magic numbers and to prevent future divergence if the limits change again.

Copilot uses AI. Check for mistakes.
}

internal bool IsAstSafe(Ast ast)
{
if ((bool)ast.Accept(this) && _visitCount < MaxVisitCount)
if ((bool)ast.Accept(this) && _visitCount < _maxVisitCount)
{
return true;
}
Expand All @@ -65,8 +69,8 @@ internal bool IsAstSafe(Ast ast)
// This is a check of the number of visits
private uint _visitCount = 0;

private const uint MaxVisitCount = 5000;
private const int MaxHashtableKeyCount = 500;
private readonly uint _maxVisitCount;
private readonly int _maxHashtableKeyCount;

// Used to determine if we are being called within a GetPowerShell() context,
// which does some additional security verification outside of the scope of
Expand Down Expand Up @@ -330,7 +334,7 @@ public object VisitArrayLiteral(ArrayLiteralAst arrayLiteralAst)

public object VisitHashtable(HashtableAst hashtableAst)
{
if (hashtableAst.KeyValuePairs.Count > MaxHashtableKeyCount)
if (hashtableAst.KeyValuePairs.Count > _maxHashtableKeyCount)
{
return false;
}
Expand Down Expand Up @@ -373,7 +377,7 @@ public static object GetSafeValue(Ast ast, ExecutionContext context, SafeValueCo
{
t_context = context;

if (safeValueContext == SafeValueContext.SkipHashtableSizeCheck || IsSafeValueVisitor.IsAstSafe(ast, safeValueContext))
if (IsSafeValueVisitor.IsAstSafe(ast, safeValueContext))
{
return ast.Accept(new GetSafeValueVisitor());
}
Expand Down
6 changes: 6 additions & 0 deletions test/powershell/Modules/Microsoft.PowerShell.Utility/PowerShellData.tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,10 @@ Describe "Tests for the Import-PowerShellDataFile cmdlet" -Tags "CI" {
$result = Import-PowerShellDataFile $largePsd1Path -SkipLimitCheck
$result.Keys.Count | Should -Be 501
}

It 'Fails if psd1 file is insecure while -SkipLimitCheck is used' {
$path = Setup -f insecure2.psd1 -Content '@{ Foo = [object] (calc.exe) }' -pass
{ Import-PowerShellDataFile $path -SkipLimitCheck -ErrorAction Stop } |
Should -Throw -ErrorId "System.InvalidOperationException,Microsoft.PowerShell.Commands.ImportPowerShellDataFileCommand"
}
}
Loading