Skip to content

Conversation

adrum
Copy link
Contributor

@adrum adrum commented May 2, 2025

When applying https://github.com/craftcms/security-patches to a Craft project, the fix for CVE-2024-56145 breaks sites served by Valet (and Herd).

This fix allows these sites to work when using Valet (and Herd).

When applying https://github.com/craftcms/security-patches to a Craft project, the fix for [CVE-2024-56145](GHSA-2p6p-9rc9-62j9) breaks sites served by Valet (and Herd). 

This fix allows these sites to work when using Valet (and Herd).
@mattstauffer
Copy link
Collaborator

@adrum Not sure if you have the details, but can you explain how argv being set breaks anything? I read through the CVE and can't easily follow why that would be the case. Thanks!

@adrum
Copy link
Contributor Author

adrum commented May 2, 2025

@mattstauffer Of course! When utilizing the security patches plugin, they are just throwing a 400 anytime it's set:

https://github.com/craftcms/security-patches/blob/main/src/Extension.php#L45

@adrum
Copy link
Contributor Author

adrum commented May 2, 2025

Here's the proper fix they applied: craftcms/cms@3.9.13...3.9.14

I think the https://github.com/craftcms/security-patches package is a quick solution that blocks the vulnerability in a "big hammer" way.

@mattstauffer
Copy link
Collaborator

Ahh, that's so helpful. So if you have this security patches thing set, then it forces this error every time. And future versions don't even do anything with argv I assume, so we can just merge this as-is.

I'm going to merge, but could you do me a sanity check and grep through the codebase to make sure there aren't any other references to argv in the codebase that we may be breaking with this? Seems unlikely...

Thanks so much!

@mattstauffer mattstauffer merged commit 6ef3b05 into laravel:master May 6, 2025
7 checks passed
@adrum
Copy link
Contributor Author

adrum commented May 9, 2025

@mattstauffer I didn't see any references, aside from vendor dependencies. All of them were CLI or mocking related:

$ grep -R argv
./cli/Valet/Drivers/Specific/CraftValetDriver.php:        if (isset($_SERVER['argv'])) {
./cli/Valet/Drivers/Specific/CraftValetDriver.php:            unset($_SERVER['argv']);
./vendor/nikic/php-parser/bin/php-parse:list($operations, $files, $attributes) = parseArgs($argv);
./vendor/mockery/mockery/library/Mockery/Generator/StringManipulation/Pass/MethodDefinitionPass.php:\$argv = func_get_args();
./vendor/mockery/mockery/library/Mockery/Generator/StringManipulation/Pass/MethodDefinitionPass.php:    \$argv[$i] = {$param};
./vendor/mockery/mockery/library/Mockery/Generator/StringManipulation/Pass/MethodDefinitionPass.php:    \$argv[$i] =& \${$param->getName()};
./vendor/mockery/mockery/library/Mockery/Generator/StringManipulation/Pass/MethodDefinitionPass.php:        $body .= "\$ret = {$invoke}(__FUNCTION__, \$argv);\n";
./vendor/phpunit/phpunit/src/TextUI/Command.php:            return (new static)->run($_SERVER['argv'], $exit);
./vendor/phpunit/phpunit/src/TextUI/Command.php:    public function run(array $argv, bool $exit = true): int
./vendor/phpunit/phpunit/src/TextUI/Command.php:        $this->handleArguments($argv);
./vendor/phpunit/phpunit/src/TextUI/Command.php:     *         return $command->run($_SERVER['argv'], $exit);
./vendor/phpunit/phpunit/src/TextUI/Command.php:    protected function handleArguments(array $argv): void
./vendor/phpunit/phpunit/src/TextUI/Command.php:            $arguments = (new Builder)->fromParameters($argv, array_keys($this->longOptions));
./vendor/phpunit/phpunit/src/Util/PHP/DefaultPhpProcess.php:            unset($env['argv'], $env['argc']);
./vendor/symfony/process/Process.php:            if (false !== $v && false === \in_array($k, ['argc', 'argv', 'ARGC', 'ARGV'], true)) {
./vendor/symfony/console/Input/ArgvInput.php: * By default, the `$_SERVER['argv']` array is used for the input values.
./vendor/symfony/console/Input/ArgvInput.php: *     $input = new ArgvInput($_SERVER['argv']);
./vendor/symfony/console/Input/ArgvInput.php: * the same rules as the argv one. It's almost always better to use the
./vendor/symfony/console/Input/ArgvInput.php:    public function __construct(array $argv = null, InputDefinition $definition = null)
./vendor/symfony/console/Input/ArgvInput.php:        $argv ??= $_SERVER['argv'] ?? [];
./vendor/symfony/console/Input/ArgvInput.php:        array_shift($argv);
./vendor/symfony/console/Input/ArgvInput.php:        $this->tokens = $argv;
./vendor/symfony/console/Input/Input.php: *  * `ArgvInput`: The input comes from the CLI arguments (argv)
./vendor/symfony/console/Completion/CompletionInput.php:     * @param string[] $tokens       the set of split tokens (e.g. COMP_WORDS or argv)
./vendor/symfony/console/SingleCommandApplication.php:        $this->setName($_SERVER['argv'][0]);
./vendor/symfony/console/Command/CompleteCommand.php:            ->addOption('input', 'i', InputOption::VALUE_REQUIRED | InputOption::VALUE_IS_ARRAY, 'An array of input tokens (e.g. COMP_WORDS or argv)')
./vendor/symfony/console/Command/CompleteCommand.php:                '  '.(string) implode(' ', $_SERVER['argv']),
./vendor/symfony/console/Command/CompleteCommand.php:        $commandName = basename($_SERVER['argv'][0]);
./vendor/symfony/console/Command/DumpCompletionCommand.php:        $commandName = basename($_SERVER['argv'][0]);
./vendor/symfony/console/Debug/CliRequest.php:            $binary = $this->server->get('argv')[0];
./vendor/sebastian/cli-parser/README.md:Library for parsing `$_SERVER['argv']`, extracted from `phpunit/phpunit`.
./vendor/sebastian/cli-parser/src/Parser.php:     * @psalm-param list<string> $argv
./vendor/sebastian/cli-parser/src/Parser.php:    public function parse(array $argv, string $shortOptions, array $longOptions = null): array
./vendor/sebastian/cli-parser/src/Parser.php:        if (empty($argv)) {
./vendor/sebastian/cli-parser/src/Parser.php:        if (isset($argv[0][0]) && $argv[0][0] !== '-') {
./vendor/sebastian/cli-parser/src/Parser.php:            array_shift($argv);
./vendor/sebastian/cli-parser/src/Parser.php:        reset($argv);
./vendor/sebastian/cli-parser/src/Parser.php:        $argv = array_map('trim', $argv);
./vendor/sebastian/cli-parser/src/Parser.php:        while (false !== $arg = current($argv)) {
./vendor/sebastian/cli-parser/src/Parser.php:            $i = key($argv);
./vendor/sebastian/cli-parser/src/Parser.php:            next($argv);
./vendor/sebastian/cli-parser/src/Parser.php:                $nonOptions = array_merge($nonOptions, array_slice($argv, $i + 1));
./vendor/sebastian/cli-parser/src/Parser.php:                    $argv
./vendor/sebastian/cli-parser/src/Parser.php:                    $argv

@adrum adrum deleted the patch-1 branch May 9, 2025 17:56
@mattstauffer
Copy link
Collaborator

@adrum brilliant. Thank you!

yCodeTech added a commit to yCodeTech/valet-windows that referenced this pull request Jun 2, 2025
…tches to Craft.

Craft errors when the `$_SERVER['argv']` variable is set, so we just need to `unset` it if it's set. More info on the Valet for Mac PR laravel#1516
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants