Skip to content

Commit fc74b38

Browse files
committed
Various Hardening Updates
Five hardening changes from a security audit. None is a fresh attacker primitive on its own; the audit recommended shipping them as one PR. - `RemoveIniEntry`: anchor the regex with `\b` and `preg_quote` the extension name, so uninstalling `foo` no longer rewrites the prefix of `extension=foo_other` lines. - `PlaceholderReplacer` and `WindowsInstall`: skip symlinks during the recursive source walk, so the iterator no longer descends into files outside the extracted source dir. - `OverrideDownloadUrlInstallListener`: warn that swapping to a release-asset URL leaves only HTTPS-to-origin as the integrity guarantee, since Composer's dist-sha was bound to the original Packagist URL. - `ConfigureOption`: validate `php-ext.configure-options[].name` against `/^[a-zA-Z][a-zA-Z0-9_-]*$/`, so configure-flag identifiers can't carry whitespace or shell metacharacters into argv or `installed.json`. - `FallbackVerificationUsingOpenSsl`: promote the fallback notice to `writeError` and document that the OpenSSL path skips Rekor transparency-log verification.
1 parent 2d228f1 commit fc74b38

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)