Skip to content

Commit 4f8a820

Browse files
GabrielDufGabrielDuf
authored
Reject shell metacharacters and use -File for PowerShell operations (#5348)
1 parent 4fb3c6d commit 4f8a820

60 files changed

Lines changed: 4274 additions & 372 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/SharedAssets/Assets/Utilities/unigetui_ps_operation.ps1‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
#Requires -Version 5
2+
# Controlled launcher for UniGetUI PowerShell package operations.
3+
#
4+
# Invoked as: powershell.exe -NoProfile -ExecutionPolicy Bypass -File <this> <mode> <command> [args...]
5+
#
6+
# This script deliberately declares NO param() block. Without one, PowerShell binds every
7+
# argument positionally into $args and performs no parameter-name binding at all, so a data
8+
# argument that happens to look like "-Mode" or "-Command" cannot be smuggled into a control
9+
# value. The arguments after <command> are splatted, which passes them to the cmdlet as data
10+
# and never re-parses them as script.
11+
12+
$ErrorActionPreference = 'Continue'
13+
$ConfirmPreference = 'None'
14+
15+
if ($args.Count -lt 2)
16+
{
17+
[Console]::Error.WriteLine('UniGetUI: the operation launcher requires a mode and a command.')
18+
exit 2
19+
}
20+
21+
$mode = [string]$args[0]
22+
$command = [string]$args[1]
23+
24+
# Windows PowerShell 5.x defaults to TLS 1.0/1.1, which the PowerShell Gallery rejects.
25+
if ($mode -eq 'tls12')
26+
{
27+
try
28+
{
29+
[Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12
30+
}
31+
catch
32+
{
33+
[Console]::Error.WriteLine('UniGetUI: could not select TLS 1.2.')
34+
}
35+
}
36+
37+
$commandInfo = $null
38+
try
39+
{
40+
$commandInfo = Get-Command -Name $command -ErrorAction Stop
41+
}
42+
catch
43+
{
44+
# Left null: without metadata nothing is coerced, and the call below reports the real error.
45+
}
46+
47+
# Whether $name names a switch on the command about to be invoked. Only a switch may have its
48+
# value coerced, because an ordinary string parameter can legitimately be given the text "$false"
49+
# - a repository named that, for instance - and must reach the cmdlet unchanged.
50+
function Test-IsSwitchParameter([string]$name)
51+
{
52+
if ($null -eq $commandInfo)
53+
{
54+
return $false
55+
}
56+
57+
$parameter = $commandInfo.Parameters[$name]
58+
if ($null -eq $parameter)
59+
{
60+
$parameter = $commandInfo.Parameters.Values |
61+
Where-Object { $_.Aliases -contains $name } |
62+
Select-Object -First 1
63+
}
64+
65+
if ($null -eq $parameter)
66+
{
67+
return $false
68+
}
69+
70+
return $parameter.ParameterType -eq [System.Management.Automation.SwitchParameter]
71+
}
72+
73+
$named = @{}
74+
$rest = @()
75+
76+
for ($i = 2; $i -lt $args.Count; $i++)
77+
{
78+
$item = [string]$args[$i]
79+
80+
# powershell.exe splits "-Switch:$false" into "-Switch" and the literal text "$false" before
81+
# this script runs, and splatting cannot bind that text to a switch. Such a pair is turned
82+
# into a real boolean and bound by name through a hashtable, which does accept one.
83+
if ($item -match '^-([A-Za-z][A-Za-z0-9_]*)$' -and ($i + 1) -lt $args.Count)
84+
{
85+
$switchName = $Matches[1]
86+
$next = [string]$args[$i + 1]
87+
88+
if (($next -eq '$false' -or $next -eq '$true') -and (Test-IsSwitchParameter $switchName))
89+
{
90+
$named[$switchName] = ($next -eq '$true')
91+
$i++
92+
continue
93+
}
94+
}
95+
96+
$rest += $args[$i]
97+
}
98+
99+
# A terminating error, such as a parameter that the cmdlet does not accept, would otherwise be
100+
# written to the error stream and leave this script to exit 0, reporting a failed operation as a
101+
# success. Running under -Command used to fail the process for us, so it is done explicitly here.
102+
try
103+
{
104+
& $command @named @rest
105+
$succeeded = $?
106+
}
107+
catch
108+
{
109+
# The whole record, not just the message: the caller matches on the error id to decide
110+
# whether to retry elevated or without -Scope, and that id is only in the full record.
111+
Write-Error -ErrorRecord $_
112+
exit 1
113+
}
114+
115+
# Running under -Command also failed the process when the command reported failure without
116+
# throwing. Not every caller binds the error variable below - PowerShell 7 operations and source
117+
# operations do not - so without this a failed operation would be reported as a success.
118+
if (-not $succeeded)
119+
{
120+
exit 1
121+
}
122+
123+
# PowerShellGet reports some failures as non-terminating errors that leave $? true, so the caller
124+
# binds -ErrorVariable to this name and it is checked as well.
125+
if ($UniGetUIOperationError)
126+
{
127+
exit 1
128+
}

‎src/UniGetUI.Avalonia/Infrastructure/ManualInstallHelper.cs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,17 @@ internal static class ManualInstallHelper
2626
{
2727
if (package is null || package.Source.IsVirtualManager) return null;
2828
var options = await InstallOptionsFactory.LoadApplicableAsync(package);
29-
var args = await Task.Run(() => package.Manager.OperationHelper.GetParameters(package, options, operation));
30-
return package.Manager.Properties.ExecutableFriendlyName + " " + string.Join(' ', args);
29+
try
30+
{
31+
var args = await Task.Run(() =>
32+
package.Manager.OperationHelper.GetStandaloneParameters(package, options, operation));
33+
return package.Manager.Properties.ExecutableFriendlyName + " " + string.Join(' ', args);
34+
}
35+
catch (InvalidOperationException ex)
36+
{
37+
Logger.Warn($"[ManualInstallHelper] No command line for {package.Id}: {ex.Message}");
38+
return null;
39+
}
3140
}
3241

3342
/// <summary>Entry point for the "Manual install/update/uninstall" menu and toolbar actions.</summary>

‎src/UniGetUI.Avalonia/UniGetUI.Avalonia.csproj‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,9 +248,12 @@
248248
<AvaloniaResource Remove="Infrastructure\**" />
249249
</ItemGroup>
250250

251+
<ItemGroup>
252+
<Content Include="..\SharedAssets\Assets\Utilities\*.ps1" Link="Assets\Utilities\%(Filename)%(Extension)" CopyToOutputDirectory="PreserveNewest" CopyToPublishDirectory="PreserveNewest" />
253+
</ItemGroup>
254+
251255
<ItemGroup Condition="$([MSBuild]::IsOSPlatform('Windows'))">
252256
<Content Include="..\SharedAssets\Assets\Utilities\*.cmd" Link="Assets\Utilities\%(Filename)%(Extension)" CopyToOutputDirectory="PreserveNewest" CopyToPublishDirectory="PreserveNewest" />
253-
<Content Include="..\SharedAssets\Assets\Utilities\*.ps1" Link="Assets\Utilities\%(Filename)%(Extension)" CopyToOutputDirectory="PreserveNewest" CopyToPublishDirectory="PreserveNewest" />
254257
<Content Include="$(ElevatorPackageExePath)" Link="Assets\Utilities\UniGetUI Elevator.exe" Condition="Exists('$(ElevatorPackageExePath)')" CopyToOutputDirectory="PreserveNewest" CopyToPublishDirectory="PreserveNewest" Visible="false" Pack="false" />
255258
<Content Include="$(ElevatorPackageDllPath)" Link="Assets\Utilities\getfilesiginforedist.dll" Condition="Exists('$(ElevatorPackageDllPath)')" CopyToOutputDirectory="PreserveNewest" CopyToPublishDirectory="PreserveNewest" Visible="false" Pack="false" />
256259
</ItemGroup>

‎src/UniGetUI.Avalonia/ViewModels/DialogPages/InstallOptionsViewModel.cs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -507,8 +507,17 @@ public async Task<string> BuildCurrentCommandAsync()
507507
var snap = SnapshotOptions();
508508
var op = CurrentOp();
509509
var applied = await InstallOptionsFactory.LoadApplicableAsync(_package, overridePackageOptions: snap);
510-
var args = await Task.Run(() => _package.Manager.OperationHelper.GetParameters(_package, applied, op));
511-
return _package.Manager.Properties.ExecutableFriendlyName + " " + string.Join(' ', args);
510+
try
511+
{
512+
var args = await Task.Run(() =>
513+
_package.Manager.OperationHelper.GetStandaloneParameters(_package, applied, op));
514+
return _package.Manager.Properties.ExecutableFriendlyName + " " + string.Join(' ', args);
515+
}
516+
catch (InvalidOperationException ex)
517+
{
518+
Logger.Warn($"[InstallOptionsViewModel] No command line for {_package.Id}: {ex.Message}");
519+
return "";
520+
}
512521
}
513522

514523
private void Refresh() { if (_uiLoaded) _ = RefreshCommandPreviewAsync(); }

‎src/UniGetUI.Avalonia/Views/SoftwarePages/PackageBundlesPage.cs‎

Lines changed: 48 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
using UniGetUI.Interface.Enums;
1414
using UniGetUI.Interface.Telemetry;
1515
using UniGetUI.PackageEngine;
16+
using UniGetUI.PackageEngine.Classes.Manager.Classes;
1617
using UniGetUI.PackageEngine.Classes.Serializable;
1718
using UniGetUI.PackageEngine.Enums;
1819
using UniGetUI.PackageEngine.Interfaces;
@@ -433,25 +434,15 @@ public static async Task<string> CreateBundle(IReadOnlyList<IPackage> unsortedPa
433434
?? throw new JsonException("Could not parse JSON object")));
434435

435436
var report = new BundleReport { IsEmpty = true };
436-
bool allowCLI = SecureSettings.Get(SecureSettings.K.AllowCLIArguments)
437-
&& SecureSettings.Get(SecureSettings.K.AllowImportingCLIArguments);
438-
bool allowPrePost = SecureSettings.Get(SecureSettings.K.AllowPrePostOpCommand)
439-
&& SecureSettings.Get(SecureSettings.K.AllowImportPrePostOpCommands);
437+
bool allowCLI = BundleImportFilter.CliArgumentsAllowed();
438+
bool allowPrePost = BundleImportFilter.PrePostCommandsAllowed();
440439

441440
var packages = new List<IPackage>();
442441
foreach (var pkg in deserializedData.packages)
443442
{
444-
var opts = pkg.InstallationOptions;
445-
ReportList(ref report, pkg.Id, opts.CustomParameters_Install, "Custom install arguments", allowCLI);
446-
ReportList(ref report, pkg.Id, opts.CustomParameters_Update, "Custom update arguments", allowCLI);
447-
ReportList(ref report, pkg.Id, opts.CustomParameters_Uninstall, "Custom uninstall arguments", allowCLI);
448-
opts.PreInstallCommand = ReportStr(ref report, pkg.Id, opts.PreInstallCommand, "Pre-install command", allowPrePost);
449-
opts.PostInstallCommand = ReportStr(ref report, pkg.Id, opts.PostInstallCommand, "Post-install command", allowPrePost);
450-
opts.PreUpdateCommand = ReportStr(ref report, pkg.Id, opts.PreUpdateCommand, "Pre-update command", allowPrePost);
451-
opts.PostUpdateCommand = ReportStr(ref report, pkg.Id, opts.PostUpdateCommand, "Post-update command", allowPrePost);
452-
opts.PreUninstallCommand = ReportStr(ref report, pkg.Id, opts.PreUninstallCommand, "Pre-uninstall command", allowPrePost);
453-
opts.PostUninstallCommand = ReportStr(ref report, pkg.Id, opts.PostUninstallCommand, "Post-uninstall command", allowPrePost);
454-
pkg.InstallationOptions = opts;
443+
pkg.InstallationOptions = BundleImportFilter.Apply(
444+
ref report, pkg.Id, pkg.InstallationOptions, allowCLI, allowPrePost,
445+
ResolveManagerForImport(pkg.ManagerName)?.CommandLineIsShellInterpreted ?? false);
455446
packages.Add(DeserializePackage(pkg));
456447
}
457448

@@ -463,16 +454,26 @@ public static async Task<string> CreateBundle(IReadOnlyList<IPackage> unsortedPa
463454
return (deserializedData.export_version, report);
464455
}
465456

466-
// ─── Deserialization helpers ──────────────────────────────────────────────
467-
public static IPackage DeserializePackage(SerializablePackage raw)
457+
private static IPackageManager? ResolveManagerForImport(string managerName)
468458
{
469-
IPackageManager? manager = null;
470-
foreach (var m in PEInterface.Managers)
459+
foreach (var manager in PEInterface.Managers)
471460
{
472-
if (m.Id == raw.ManagerName || m.Name == raw.ManagerName || m.DisplayName == raw.ManagerName)
473-
{ manager = m; break; }
461+
if (
462+
manager.Id == managerName
463+
|| manager.Name == managerName
464+
|| manager.DisplayName == managerName
465+
)
466+
return manager;
474467
}
475468

469+
return null;
470+
}
471+
472+
// ─── Deserialization helpers ──────────────────────────────────────────────
473+
public static IPackage DeserializePackage(SerializablePackage raw)
474+
{
475+
IPackageManager? manager = ResolveManagerForImport(raw.ManagerName);
476+
476477
IManagerSource? source;
477478
if (manager?.Capabilities.SupportsCustomSources == true)
478479
{
@@ -492,25 +493,6 @@ public static IPackage DeserializePackage(SerializablePackage raw)
492493
public static IPackage DeserializeIncompatiblePackage(SerializableIncompatiblePackage raw, IManagerSource source)
493494
=> new InvalidImportedPackage(raw, source);
494495

495-
// ─── Security report helpers ──────────────────────────────────────────────
496-
private static void ReportList(ref BundleReport report, string id, List<string> values, string label, bool allowed)
497-
{
498-
if (!values.Any(x => x.Any())) return;
499-
if (!report.Contents.ContainsKey(id)) report.Contents[id] = [];
500-
report.Contents[id].Add(new BundleReportEntry($"{label}: [{string.Join(", ", values)}]", allowed));
501-
report.IsEmpty = false;
502-
if (!allowed) values.Clear();
503-
}
504-
505-
private static string ReportStr(ref BundleReport report, string id, string value, string label, bool allowed)
506-
{
507-
if (!value.Any()) return value;
508-
if (!report.Contents.ContainsKey(id)) report.Contents[id] = [];
509-
report.Contents[id].Add(new BundleReportEntry($"{label}: {value}", allowed));
510-
report.IsEmpty = false;
511-
return allowed ? value : "";
512-
}
513-
514496
// ─── Batch script export ──────────────────────────────────────────────────
515497
private async Task CreateBatchScriptAsync()
516498
{
@@ -539,16 +521,39 @@ private async Task CreateBatchScriptAsync()
539521
{
540522
if (p is not ImportedPackage pkg) continue;
541523

524+
// Resolved before anything is appended: a package whose command line is refused
525+
// must contribute nothing at all, or its imported pre-install command would still
526+
// run from the script even though the install itself was dropped.
527+
IReadOnlyList<string> param;
528+
try
529+
{
530+
param = pkg.Manager.OperationHelper.GetStandaloneParameters(
531+
pkg, pkg.installation_options, OperationType.Install);
532+
}
533+
catch (InvalidOperationException ex)
534+
{
535+
Logger.Warn($"Skipping {pkg.Id} in the exported script: {ex.Message}");
536+
continue;
537+
}
538+
542539
packages.Add(pkg.Name + " from " + pkg.Manager.DisplayName);
543540

544541
foreach (var process in pkg.installation_options.KillBeforeOperation)
542+
{
543+
// Refused rather than stripped: dropping a character would silently retarget
544+
// the kill at whatever process the shortened name happens to match.
545+
if (!CoreTools.IsSafeProcessImageName(process))
546+
{
547+
Logger.Warn(
548+
$"Skipping the process \"{process}\" of {pkg.Id} in the exported script: it is not a usable process name.");
549+
continue;
550+
}
545551
commands.Add($"taskkill /im \"{process}\"" + (forceKill ? " /f" : ""));
552+
}
546553

547554
if (pkg.installation_options.PreInstallCommand != "")
548555
commands.Add(pkg.installation_options.PreInstallCommand);
549556

550-
var param = pkg.Manager.OperationHelper.GetParameters(
551-
pkg, pkg.installation_options, OperationType.Install);
552557
commands.Add($"{pkg.Manager.Properties.ExecutableFriendlyName} {string.Join(' ', param)}");
553558

554559
if (pkg.installation_options.PostInstallCommand != "")
@@ -604,7 +609,7 @@ private static string GenerateCommandString(IReadOnlyList<string> names, IReadOn
604609
if ($args[0] -ne "/DisablePausePrompts") { pause }
605610
Write-Host ""
606611
Write-Host "This script will attempt to install the following packages:"
607-
{{string.Join('\n', names.Select(x => $"Write-Host \" - {x}\""))}}
612+
{{string.Join('\n', names.Select(x => $"Write-Host {CoreTools.EscapePowerShellSingleQuoted($" - {x}")}"))}}
608613
Write-Host ""
609614
if ($args[0] -ne "/DisablePausePrompts") { pause }
610615
Clear-Host

‎src/UniGetUI.Core.Data/CoreData.cs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -576,6 +576,20 @@ private static int GetCodePage()
576576
? Path.Join(Environment.SystemDirectory, "windowspowershell\\v1.0\\powershell.exe")
577577
: "pwsh";
578578

579+
/// <summary>
580+
/// The controlled launcher script that runs PowerShell package operations. It lives next to
581+
/// the application binary so it carries the same integrity as the executable itself, and it
582+
/// is invoked with -File so the operation parameters bind as data instead of being
583+
/// reassembled into a script body the way -Command does.
584+
/// </summary>
585+
public static string PowerShellOperationLauncher =>
586+
Path.Join(
587+
UniGetUIExecutableDirectory,
588+
"Assets",
589+
"Utilities",
590+
"unigetui_ps_operation.ps1"
591+
);
592+
579593
private static string GetLocalDataRoot()
580594
{
581595
string localApplicationData = Environment.GetFolderPath(

0 commit comments

Comments
 (0)