Skip to content

Commit 5d1485c

Browse files
asgrimasgrim
committed
Merge up 1.3.x to 1.4.x for security vulnerability patches
2 parents a6873a3 + f5203dc commit 5d1485c

30 files changed

Lines changed: 1018 additions & 288 deletions

‎.github/workflows/build-assets.yml‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ jobs:
2626
php-versions:
2727
- '8.1'
2828
permissions:
29+
contents: read
2930
# id-token:write is required for build provenance attestation.
3031
id-token: write
3132
# attestations:write is required for build provenance attestation.
@@ -55,7 +56,7 @@ jobs:
5556
# unprivileged context, otherwise anyone could send a PR with malicious
5657
# code, which would store attestation that `php/pie` built the PHAR, and
5758
# it would look genuine. So this should NOT run for PR builds.
58-
if: github.event_name != 'pull_request'
59+
if: github.event_name != 'pull_request' && github.event.repository.visibility == 'public'
5960
uses: actions/attest@v4
6061
with:
6162
subject-path: '${{ github.workspace }}/pie.phar'
@@ -80,6 +81,7 @@ jobs:
8081
- macos-26
8182
# - windows-2025 - disabled for now, seems broken
8283
permissions:
84+
contents: read
8385
# id-token:write is required for build provenance attestation.
8486
id-token: write
8587
# attestations:write is required for build provenance attestation.
@@ -156,7 +158,7 @@ jobs:
156158
# unprivileged context, otherwise anyone could send a PR with malicious
157159
# code, which would store attestation that `php/pie` built the binaries,
158160
# and it would look genuine. So this should NOT run for PR builds.
159-
if: github.event_name != 'pull_request'
161+
if: github.event_name != 'pull_request' && github.event.repository.visibility == 'public'
160162
uses: actions/attest@v4
161163
with:
162164
subject-path: '${{ github.workspace }}/${{ env.PIE_BINARY_OUTPUT }}'

‎phpstan-baseline.neon‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,10 @@ parameters:
5555
path: src/Command/RepositoryRemoveCommand.php
5656

5757
-
58-
message: '#^Parameter \#2 \$content of static method Php\\Pie\\File\\SudoFilePut\:\:contents\(\) expects string, string\|false given\.$#'
58+
message: '#^Parameter \#1 \$tag of class Php\\Pie\\SelfManage\\Update\\ReleaseMetadata constructor expects non\-empty\-string, mixed given\.$#'
5959
identifier: argument.type
6060
count: 1
61-
path: src/Command/SelfUpdateCommand.php
61+
path: src/Command/SelfVerifyCommand.php
6262

6363
-
6464
message: '#^Cannot cast mixed to string\.$#'
@@ -447,13 +447,13 @@ parameters:
447447
-
448448
message: '#^Parameter \#1 \$certificate of function openssl_x509_export expects OpenSSLCertificate\|string, OpenSSLCertificate\|false given\.$#'
449449
identifier: argument.type
450-
count: 2
450+
count: 4
451451
path: test/unit/SelfManage/Verify/FallbackVerificationUsingOpenSslTest.php
452452

453453
-
454454
message: '#^Parameter \#1 \$csr of function openssl_csr_sign expects OpenSSLCertificateSigningRequest\|string, OpenSSLCertificateSigningRequest\|false given\.$#'
455455
identifier: argument.type
456-
count: 2
456+
count: 4
457457
path: test/unit/SelfManage/Verify/FallbackVerificationUsingOpenSslTest.php
458458

459459
-
@@ -463,43 +463,61 @@ parameters:
463463
path: test/unit/SelfManage/Verify/FallbackVerificationUsingOpenSslTest.php
464464

465465
-
466-
message: '#^Parameter \#1 \$string of function trim expects string, array\<string\>\|string given\.$#'
466+
message: '#^Parameter \#1 \$string of function strlen expects string, string\|false given\.$#'
467467
identifier: argument.type
468468
count: 1
469469
path: test/unit/SelfManage/Verify/FallbackVerificationUsingOpenSslTest.php
470470

471+
-
472+
message: '#^Parameter \#1 \$string of function trim expects string, array\<string\>\|string given\.$#'
473+
identifier: argument.type
474+
count: 2
475+
path: test/unit/SelfManage/Verify/FallbackVerificationUsingOpenSslTest.php
476+
471477
-
472478
message: '#^Parameter \#2 \$ca_certificate of function openssl_csr_sign expects OpenSSLCertificate\|string\|null, OpenSSLCertificate\|false given\.$#'
473479
identifier: argument.type
474-
count: 1
480+
count: 2
475481
path: test/unit/SelfManage/Verify/FallbackVerificationUsingOpenSslTest.php
476482

477483
-
478484
message: '#^Parameter \#2 \$dsseEnvelopePayload of method Php\\PieUnitTest\\SelfManage\\Verify\\FallbackVerificationUsingOpenSslTest\:\:mockAttestationResponse\(\) expects string, string\|false given\.$#'
479485
identifier: argument.type
480-
count: 3
486+
count: 4
481487
path: test/unit/SelfManage/Verify/FallbackVerificationUsingOpenSslTest.php
482488

483489
-
484490
message: '#^Parameter \#3 \$private_key of function openssl_csr_sign expects array\|OpenSSLAsymmetricKey\|OpenSSLCertificate\|string, mixed given\.$#'
485491
identifier: argument.type
486-
count: 2
492+
count: 4
487493
path: test/unit/SelfManage/Verify/FallbackVerificationUsingOpenSslTest.php
488494

489495
-
490496
message: '#^Parameter \#3 \$private_key of function openssl_sign expects array\|OpenSSLAsymmetricKey\|OpenSSLCertificate\|string, mixed given\.$#'
491497
identifier: argument.type
498+
count: 2
499+
path: test/unit/SelfManage/Verify/FallbackVerificationUsingOpenSslTest.php
500+
501+
-
502+
message: '#^Parameter \#3 \$signature of method Php\\PieUnitTest\\SelfManage\\Verify\\FallbackVerificationUsingOpenSslTest\:\:mockAttestationResponse\(\) expects string, mixed given\.$#'
503+
identifier: argument.type
492504
count: 1
493505
path: test/unit/SelfManage/Verify/FallbackVerificationUsingOpenSslTest.php
494506

495507
-
496508
message: '#^Parameter \#3 \$subject of function str_replace expects array\<string\>\|string, mixed given\.$#'
497509
identifier: argument.type
498-
count: 1
510+
count: 2
499511
path: test/unit/SelfManage/Verify/FallbackVerificationUsingOpenSslTest.php
500512

501513
-
502514
message: '#^Parameter \#4 \$body of class Composer\\Util\\Http\\Response constructor expects string\|null, string\|false given\.$#'
503515
identifier: argument.type
504516
count: 1
505517
path: test/unit/SelfManage/Verify/FallbackVerificationUsingOpenSslTest.php
518+
519+
-
520+
message: '#^Parameter \#4 \$pemCertificate of method Php\\PieUnitTest\\SelfManage\\Verify\\FallbackVerificationUsingOpenSslTest\:\:mockAttestationResponse\(\) expects string, mixed given\.$#'
521+
identifier: argument.type
522+
count: 1
523+
path: test/unit/SelfManage/Verify/FallbackVerificationUsingOpenSslTest.php

‎src/Command/SelfUpdateCommand.php‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
use Composer\IO\IOInterface;
88
use Composer\IO\NullIO;
9-
use Composer\Util\HttpDownloader;
109
use Php\Pie\ComposerIntegration\PieComposerFactory;
1110
use Php\Pie\ComposerIntegration\PieComposerRequest;
1211
use Php\Pie\ComposerIntegration\QuieterConsoleIO;
@@ -116,9 +115,12 @@ public function execute(InputInterface $input, OutputInterface $output): int
116115
),
117116
);
118117

119-
$httpDownloader = new HttpDownloader($this->quieterConsoleIo, $composer->getConfig());
120-
$fetchLatestPieRelease = new FetchPieReleaseFromGitHub($this->githubApiBaseUrl, $httpDownloader);
121-
$verifyPiePhar = VerifyPieReleaseUsingAttestation::factory();
118+
$fetchLatestPieRelease = FetchPieReleaseFromGitHub::factory(
119+
$this->quieterConsoleIo,
120+
$composer->getConfig(),
121+
$this->githubApiBaseUrl,
122+
);
123+
$verifyPiePhar = VerifyPieReleaseUsingAttestation::factory($fetchLatestPieRelease);
122124

123125
if ($updateChannel === Channel::Nightly) {
124126
$latestRelease = new ReleaseMetadata(
@@ -177,12 +179,30 @@ public function execute(InputInterface $input, OutputInterface $output): int
177179
return Command::FAILURE;
178180
}
179181

182+
$pharContents = file_get_contents($pharFilename->filePath);
183+
184+
if ($pharContents === false) {
185+
$this->io->writeError(sprintf('<error>%s Failed to read the downloaded PHAR file %s</error>', Emoji::CROSS, $pharFilename->filePath));
186+
unlink($pharFilename->filePath);
187+
188+
return Command::FAILURE;
189+
}
190+
191+
try {
192+
$pharFilename->verifyContent($pharContents);
193+
} catch (Throwable) {
194+
$this->io->writeError(sprintf('<error>%s PHAR contents changed after verification; aborting self-update</error>', Emoji::CROSS));
195+
unlink($pharFilename->filePath);
196+
197+
return Command::FAILURE;
198+
}
199+
180200
$fullPathToSelf = ($this->fullPathToSelf)();
181201
$this->io->write(
182202
sprintf('Writing new version to %s', $fullPathToSelf),
183203
verbosity: IOInterface::VERBOSE,
184204
);
185-
SudoFilePut::contents($fullPathToSelf, file_get_contents($pharFilename->filePath));
205+
SudoFilePut::contents($fullPathToSelf, $pharContents);
186206
unlink($pharFilename->filePath);
187207

188208
$this->io->write(sprintf(

‎src/Command/SelfVerifyCommand.php‎

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,22 @@
55
namespace Php\Pie\Command;
66

77
use Composer\IO\IOInterface;
8+
use Composer\IO\NullIO;
9+
use Php\Pie\ComposerIntegration\PieComposerFactory;
10+
use Php\Pie\ComposerIntegration\PieComposerRequest;
11+
use Php\Pie\ComposerIntegration\QuieterConsoleIO;
812
use Php\Pie\File\BinaryFile;
913
use Php\Pie\File\FullPathToSelf;
14+
use Php\Pie\SelfManage\Update\FetchPieReleaseFromGitHub;
1015
use Php\Pie\SelfManage\Update\ReleaseMetadata;
1116
use Php\Pie\SelfManage\Verify\FailedToVerifyRelease;
1217
use Php\Pie\SelfManage\Verify\VerifyPieReleaseUsingAttestation;
1318
use Php\Pie\Util\Emoji;
1419
use Php\Pie\Util\PieVersion;
20+
use Psr\Container\ContainerInterface;
1521
use Symfony\Component\Console\Attribute\AsCommand;
1622
use Symfony\Component\Console\Command\Command;
23+
use Symfony\Component\Console\Input\InputArgument;
1724
use Symfony\Component\Console\Input\InputInterface;
1825
use Symfony\Component\Console\Output\OutputInterface;
1926

@@ -25,9 +32,15 @@
2532
)]
2633
final class SelfVerifyCommand extends Command
2734
{
35+
private const ARGUMENT_VERSION = 'version';
36+
37+
/** @param non-empty-string $githubApiBaseUrl */
2838
public function __construct(
39+
private readonly string $githubApiBaseUrl,
2940
private readonly FullPathToSelf $fullPathToSelf,
3041
private readonly IOInterface $io,
42+
private readonly QuieterConsoleIO $quieterConsoleIo,
43+
private readonly ContainerInterface $container,
3144
) {
3245
parent::__construct();
3346
}
@@ -37,6 +50,11 @@ public function configure(): void
3750
parent::configure();
3851

3952
CommandHelper::configurePhpConfigOptions($this);
53+
$this->addArgument(
54+
self::ARGUMENT_VERSION,
55+
InputArgument::OPTIONAL,
56+
'The version of PIE you expect to be running (e.g. 1.4.4 or nightly)',
57+
);
4058
}
4159

4260
public function execute(InputInterface $input, OutputInterface $output): int
@@ -47,15 +65,40 @@ public function execute(InputInterface $input, OutputInterface $output): int
4765
return Command::FAILURE;
4866
}
4967

50-
$latestRelease = new ReleaseMetadata(PieVersion::get(), 'blah');
68+
$expectedVersion = $input->getArgument(self::ARGUMENT_VERSION);
69+
70+
if ($expectedVersion === null) {
71+
$expectedVersion = PieVersion::get();
72+
$this->io->write(sprintf('<comment>No version specified, verifying against the version this PHAR claims to be (%s).</comment>', $expectedVersion));
73+
}
74+
75+
$targetPlatform = CommandHelper::determineTargetPlatformFromInputs($input, $this->io);
76+
77+
CommandHelper::applyNoCacheOptionIfSet($input, $this->io);
78+
79+
$composer = PieComposerFactory::createPieComposer(
80+
$this->container,
81+
PieComposerRequest::noOperation(
82+
new NullIO(),
83+
$targetPlatform,
84+
),
85+
);
86+
87+
$fetchLatestPieRelease = FetchPieReleaseFromGitHub::factory(
88+
$this->quieterConsoleIo,
89+
$composer->getConfig(),
90+
$this->githubApiBaseUrl,
91+
);
92+
93+
$latestRelease = new ReleaseMetadata($expectedVersion, 'blah');
5194
$pharFilename = BinaryFile::fromFileWithSha256Checksum(($this->fullPathToSelf)());
52-
$verifyPiePhar = VerifyPieReleaseUsingAttestation::factory();
95+
$verifyPiePhar = VerifyPieReleaseUsingAttestation::factory($fetchLatestPieRelease);
5396

5497
try {
5598
$verifyPiePhar->verify($latestRelease, $pharFilename, $this->io);
5699
} catch (FailedToVerifyRelease $failedToVerifyRelease) {
57100
$this->io->writeError(sprintf(
58-
'<error>❌ Failed to verify the pie.phar release %s: %s</error>',
101+
'<error>❌ Failed to verify that this PIE binary is the authentic release %s: %s</error>',
59102
$latestRelease->tag,
60103
$failedToVerifyRelease->getMessage(),
61104
));
@@ -64,7 +107,7 @@ public function execute(InputInterface $input, OutputInterface $output): int
64107
}
65108

66109
$this->io->write(sprintf(
67-
'<info>%s You are running an authentic PIE version %s.</info>',
110+
'<info>%s This is an authentic PIE release for version %s.</info>',
68111
Emoji::GREEN_CHECKMARK,
69112
$latestRelease->tag,
70113
));

‎src/ComposerIntegration/UninstallProcess.php‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,12 @@ public function __invoke(
2828
PieComposerRequest $composerRequest,
2929
CompletePackageInterface $composerPackage,
3030
): void {
31-
$io = $composerRequest->pieOutput;
31+
$io = $composerRequest->pieOutput;
32+
$targetPlatform = $composerRequest->targetPlatform;
3233

3334
$piePackage = Package::fromComposerCompletePackage($composerPackage);
3435

35-
$affectedIniFiles = ($this->removeIniEntry)($piePackage, $composerRequest->targetPlatform, $io);
36+
$affectedIniFiles = ($this->removeIniEntry)($piePackage, $targetPlatform, $io);
3637

3738
if (count($affectedIniFiles) === 1) {
3839
$io->write(
@@ -52,6 +53,6 @@ public function __invoke(
5253
array_walk($affectedIniFiles, static fn (string $ini) => $io->write(' - ' . $ini));
5354
}
5455

55-
$io->write(sprintf('👋 <info>Removed extension:</info> %s', ($this->uninstall)($piePackage)->filePath));
56+
$io->write(sprintf('👋 <info>Removed extension:</info> %s', ($this->uninstall)($targetPlatform, $piePackage)->filePath));
5657
}
5758
}

‎src/DependencyResolver/Package.php‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
use function parse_url;
3131
use function str_contains;
3232
use function str_starts_with;
33+
use function strlen;
3334
use function strtolower;
3435

3536
use const DIRECTORY_SEPARATOR;
@@ -86,7 +87,23 @@ public static function fromComposerCompletePackage(CompletePackageInterface $com
8687

8788
$package->supportZts = $phpExtOptions['support-zts'] ?? true;
8889
$package->supportNts = $phpExtOptions['support-nts'] ?? true;
89-
$package->buildPath = $phpExtOptions['build-path'] ?? null;
90+
91+
$buildPath = $phpExtOptions['build-path'] ?? null;
92+
if ($buildPath !== null) {
93+
if (
94+
str_starts_with($buildPath, '/')
95+
|| str_starts_with($buildPath, '\\')
96+
|| (strlen($buildPath) > 1 && $buildPath[1] === ':')
97+
) {
98+
throw new InvalidArgumentException('php-ext.build-path must be a relative path.');
99+
}
100+
101+
if (str_contains($buildPath, '..')) {
102+
throw new InvalidArgumentException('php-ext.build-path cannot contain ".." segments.');
103+
}
104+
}
105+
106+
$package->buildPath = $buildPath;
90107

91108
$compatibleOsFamilies = $phpExtOptions['os-families'] ?? null;
92109
$incompatibleOsFamilies = $phpExtOptions['os-families-exclude'] ?? null;

‎src/Downloading/DownloadedPackage.php‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
use Php\Pie\DependencyResolver\Package;
88
use Php\Pie\Platform\PrePackagedSourceAssetName;
9+
use RuntimeException;
910

1011
use function array_map;
1112
use function array_unique;
@@ -14,7 +15,9 @@
1415
use function is_string;
1516
use function pathinfo;
1617
use function realpath;
18+
use function sprintf;
1719
use function str_replace;
20+
use function str_starts_with;
1821

1922
use const DIRECTORY_SEPARATOR;
2023
use const PATHINFO_FILENAME;
@@ -71,17 +74,27 @@ private static function overrideSourcePathUsingBuildPath(Package $package, strin
7174
return $extractedSourcePath;
7275
}
7376

74-
$extractedSourcePathWithBuildPath = realpath(
77+
$candidate = realpath(
7578
$extractedSourcePath
7679
. DIRECTORY_SEPARATOR
7780
. str_replace('{version}', $package->version(), $package->buildPath()),
7881
);
7982

80-
if (! is_string($extractedSourcePathWithBuildPath)) {
83+
if (! is_string($candidate)) {
8184
return $extractedSourcePath;
8285
}
8386

84-
return $extractedSourcePathWithBuildPath;
87+
$extractedReal = realpath($extractedSourcePath);
88+
if ($extractedReal === false || ! str_starts_with($candidate . DIRECTORY_SEPARATOR, $extractedReal . DIRECTORY_SEPARATOR)) {
89+
throw new RuntimeException(sprintf(
90+
'php-ext.build-path %s resolved to %s, which is outside the extract directory %s',
91+
$package->buildPath(),
92+
$candidate,
93+
$extractedReal,
94+
));
95+
}
96+
97+
return $candidate;
8598
}
8699

87100
public static function fromPackageAndExtractedPath(Package $package, string $extractedSourcePath): self

0 commit comments

Comments
 (0)