admin/PowerShell-PowerShell
1
0
Fork 0
mirror of https://github.com/PowerShell/PowerShell synced 2026-06-08 12:12:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merged PR 38882: Merged PR 38690: Update MaxVisitCount and MaxHashtableKeyCount if visitor saf...

Browse Source 
Merged PR 38690: Update MaxVisitCount and MaxHashtableKeyCount if visitor safe value context indicates SkipLimitCheck is true

----
#### AI description  (iteration 1)
#### PR Classification
Bug fix that updates the safe value checks by dynamically adjusting limit values based on the visitor context to improve security.

#### PR Summary
This pull request refactors the safe value visitor logic to replace hard-coded limit constants with context-driven readonly fields and adjusts the conditional checks accordingly. It also adds a test to verify that insecure psd1 files correctly fail when the -SkipLimitCheck parameter is used.
- `src/System.Management.Automation/engine/parser/SafeValues.cs`: Replaces constant limits with dynamic fields (_maxVisitCount and _maxHashtableKeyCount) set based on the safe value context, and updates conditional checks to use these fields.
- `test/powershell/Modules/Microsoft.PowerShell.Utility/PowerShellData.tests.ps1`: Adds a test case to ensure that insecure psd1 files trigger an error when -SkipLimitCheck is applied.
<!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot -->

Related work items: #163511
This commit is contained in:
Justin Chung
2026-03-10 17:45:10 +00:00
committed by Travis Plunk (He Him)
parent 2a4949b20a
commit 62c12ee7e5
2 changed files with 15 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/System.Management.Automation/engine/parser/SafeValues.cs
+9 -5
View File
@@ -47,11 +47,15 @@ namespace System.Management.Automation.Language
internal IsSafeValueVisitor(GetSafeValueVisitor.SafeValueContext safeValueContext)
{
_safeValueContext = safeValueContext;
bool skipSizeCheck = safeValueContext is GetSafeValueVisitor.SafeValueContext.SkipHashtableSizeCheck;
_maxVisitCount = skipSizeCheck ? uint.MaxValue : 5000;
_maxHashtableKeyCount = skipSizeCheck ? int.MaxValue : 500;
}
internal bool IsAstSafe(Ast ast)
{
if ((bool)ast.Accept(this) && _visitCount < MaxVisitCount)
if ((bool)ast.Accept(this) && _visitCount < _maxVisitCount)
{
return true;
}
@@ -65,8 +69,8 @@ namespace System.Management.Automation.Language
// 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
@@ -330,7 +334,7 @@ namespace System.Management.Automation.Language
public object VisitHashtable(HashtableAst hashtableAst)
{
if (hashtableAst.KeyValuePairs.Count > MaxHashtableKeyCount)
if (hashtableAst.KeyValuePairs.Count > _maxHashtableKeyCount)
{
return false;
}
@@ -373,7 +377,7 @@ namespace System.Management.Automation.Language
{
t_context = context;
if (safeValueContext == SafeValueContext.SkipHashtableSizeCheck || IsSafeValueVisitor.IsAstSafe(ast, safeValueContext))
if (IsSafeValueVisitor.IsAstSafe(ast, safeValueContext))
{
return ast.Accept(new GetSafeValueVisitor());
}
test/powershell/Modules/Microsoft.PowerShell.Utility/PowerShellData.tests.ps1
+6
View File
@@ -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"
}
}