Skip to content

Commit 51e0dc1

Browse files
authored
Merge pull request #617 from iliaal/hardening-various
Various Hardening Updates
2 parents 2d228f1 + fc74b38 commit 51e0dc1

6 files changed

Lines changed: 41 additions & 6 deletions

File tree

src/Building/PlaceholderReplacer.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,13 @@ public function replacePlaceholdersWithPlaceholderReplacements(IOInterface $io,
5757
foreach (new RecursiveIteratorIterator(new RecursiveDirectoryIterator($downloadedPackage->extractedSourcePath)) as $file) {
5858
assert($file instanceof SplFileInfo);
5959

60+
// Refuse to follow symlinks into files the package author did not
61+
// legitimately own at build time (e.g., an archive entry that points
62+
// at the invoking user's $HOME).
63+
if ($file->isLink()) {
64+
continue;
65+
}
66+
6067
if (! $file->isFile() || ! in_array($file->getExtension(), self::FILE_EXTENSIONS)) {
6168
continue;
6269
}

src/ComposerIntegration/Listeners/OverrideDownloadUrlInstallListener.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,15 @@ function (OperationInterface $operation): void {
129129
$this->composerRequest->pieOutput->write('Found prebuilt archive: ' . $url);
130130
$composerPackage->setDistUrl($url);
131131

132+
// Composer's dist-sha was computed against the original
133+
// Packagist URL; once we swap to a release-asset URL the
134+
// FileDownloader has nothing to validate the new bytes
135+
// against. Surface that so the caller knows HTTPS-to-origin
136+
// is the only integrity guarantee left.
137+
$this->composerRequest->pieOutput->write(
138+
'<warning>Note: dist-sha integrity check is not available for prebuilt-binary URLs; HTTPS to the release-asset origin is the only integrity guarantee.</warning>',
139+
);
140+
132141
if (pathinfo($url, PATHINFO_EXTENSION) === 'tgz') {
133142
$composerPackage->setDistType('tar');
134143
}

src/ConfigureOption.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,14 @@ public static function fromComposerJsonDefinition(array $configureOptionDefiniti
2828
{
2929
Assert::keyExists($configureOptionDefinition, 'name');
3030
Assert::stringNotEmpty($configureOptionDefinition['name']);
31+
// Restrict to identifier characters that match real ./configure flag
32+
// conventions. Whitespace and shell metacharacters in this field flow
33+
// verbatim into argv, log output, and installed.json metadata.
34+
Assert::regex(
35+
$configureOptionDefinition['name'],
36+
'/^[a-zA-Z][a-zA-Z0-9_-]*$/',
37+
'php-ext.configure-options[].name must be a configure-flag identifier (got %s)',
38+
);
3139

3240
$needsValue = false;
3341
if (array_key_exists('needs-value', $configureOptionDefinition)) {

src/Installing/Ini/RemoveIniEntryWithFileGetContents.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use function file_exists;
2020
use function in_array;
2121
use function is_dir;
22+
use function preg_quote;
2223
use function Safe\file_get_contents;
2324
use function Safe\preg_replace;
2425
use function Safe\scandir;
@@ -67,10 +68,14 @@ static function (string $path) use ($additionalIniDirectory): bool {
6768
// Make sure all symlinks are resolved
6869
$allIniFiles = array_filter(array_map('realpath', $allIniFiles));
6970

71+
// Anchor on the right with \b so uninstalling `foo` doesn't also
72+
// rewrite the prefix of `extension=foo_other`. preg_quote on the
73+
// extension name is defence-in-depth in case future ExtensionName
74+
// validation ever loosens past `^[A-Za-z][a-zA-Z0-9_]+$`.
7075
$regex = sprintf(
71-
'/^(%s\s*=\s*%s)/m',
76+
'/^(%s\s*=\s*%s)\b/m',
7277
$package->extensionType() === ExtensionType::PhpModule ? 'extension' : 'zend_extension',
73-
$package->extensionName()->name(),
78+
preg_quote($package->extensionName()->name(), '/'),
7479
);
7580

7681
$updatedIniFiles = [];

src/Installing/WindowsInstall.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,13 @@ public function __invoke(
5858
assert($file instanceof SplFileInfo);
5959

6060
/**
61-
* Skip directories, the main DLL, PDB
61+
* Skip directories, the main DLL, PDB, and any symlinks the archive
62+
* may have shipped (symlink-followed targets fall outside the source
63+
* dir's containment guarantees).
6264
*/
6365
if (
6466
$file->isDir()
67+
|| $file->isLink()
6568
|| $this->normalisedPathsMatch($file->getPathname(), $sourceDllName)
6669
|| $this->normalisedPathsMatch($file->getPathname(), $sourcePdbName)
6770
) {

src/SelfManage/Verify/FallbackVerificationUsingOpenSsl.php

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,12 @@ public function __construct(
3636

3737
public function verify(ReleaseMetadata $releaseMetadata, BinaryFile $pharFilename, IOInterface $io): void
3838
{
39-
$io->write(
40-
'Falling back to basic verification. To use full verification, install the `gh` CLI tool.',
41-
verbosity: IOInterface::VERBOSE,
39+
// The fallback verifier checks cert chain, cert extension claims, DSSE
40+
// subject digest, and DSSE signature, but does NOT validate Rekor
41+
// transparency-log inclusion. `gh attestation verify` does. Surface the
42+
// reduced guarantees so users on shared / air-gapped hosts know.
43+
$io->writeError(
44+
'<warning>Falling back to OpenSSL verification (no Rekor inclusion check). Install `gh` for full attestation verification.</warning>',
4245
);
4346

4447
try {

0 commit comments

Comments
 (0)