Skip to content

Commit 70b6307

Browse files
svpernova09svpernova09
committed
Fix downloads.php crash on invalid os parameter
 Validate the client-supplied ?os= against the whitelist before indexing, fixing ~24k/day 'array_key_exists(): ... null given' fatals. Extract the resolution into a testable OptionResolver class.
1 parent 9ffddc4 commit 70b6307

3 files changed

Lines changed: 230 additions & 37 deletions

File tree

‎downloads.php‎

Lines changed: 7 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
<?php
2+
use phpweb\Downloads\OptionResolver;
3+
24
$_SERVER['BASE_PAGE'] = 'downloads.php';
35
include_once __DIR__ . '/include/prepend.inc';
46
include_once __DIR__ . '/include/gpg-keys.inc';
@@ -99,43 +101,11 @@ function option(string $value, string $desc, $attributes = []): string
99101
];
100102

101103

102-
$platform = $_SERVER['HTTP_SEC_CH_UA_PLATFORM'] ?? '';
103-
$ua = $_SERVER['HTTP_USER_AGENT'] ?? '';
104-
$auto_os = null;
105-
$auto_osvariant = null;
106-
107-
if (!empty($platform) || !empty($ua)) {
108-
$platform = strtolower(trim($platform, '"'));
109-
if ($platform === 'windows' || stripos($ua, 'Windows') !== false) {
110-
$auto_os = 'windows';
111-
} elseif ($platform === 'macos' || stripos($ua, 'Mac') !== false) {
112-
$auto_os = 'osx';
113-
} elseif ($platform === 'linux' || stripos($ua, 'Linux') !== false) {
114-
$auto_os = 'linux';
115-
if (stripos($ua, 'Ubuntu') !== false) {
116-
$auto_osvariant = 'linux-ubuntu';
117-
} elseif (stripos($ua, 'Debian') !== false) {
118-
$auto_osvariant = 'linux-debian';
119-
} elseif (stripos($ua, 'Fedora') !== false) {
120-
$auto_osvariant = 'linux-fedora';
121-
} elseif (stripos($ua, 'Red Hat') !== false || stripos($ua, 'RedHat') !== false) {
122-
$auto_osvariant = 'linux-redhat';
123-
}
124-
}
125-
}
126-
127-
$defaults = [
128-
'os' => $auto_os ?? 'linux',
129-
'version' => 'default',
130-
];
131-
132-
$options = array_merge($defaults, $_GET);
133-
134-
if ($auto_osvariant && (!array_key_exists('osvariant', $options) || !array_key_exists($options['osvariant'], $os[$options['os']]['variants']))) {
135-
$options['osvariant'] = $auto_osvariant;
136-
} elseif (!array_key_exists('osvariant', $options) || !array_key_exists($options['osvariant'], $os[$options['os']]['variants'])) {
137-
$options['osvariant'] = array_key_first($os[$options['os']]['variants']);
138-
}
104+
$options = (new OptionResolver($os))->resolve(
105+
$_GET,
106+
$_SERVER['HTTP_SEC_CH_UA_PLATFORM'] ?? '',
107+
$_SERVER['HTTP_USER_AGENT'] ?? '',
108+
);
139109
?>
140110
<h1>Downloads &amp; Installation Instructions</h1>
141111

‎src/Downloads/OptionResolver.php‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace phpweb\Downloads;
6+
7+
class OptionResolver
8+
{
9+
/**
10+
* @param array<string, array{name: string, variants: array<string, string>}> $osList
11+
*/
12+
public function __construct(private readonly array $osList)
13+
{
14+
}
15+
16+
/**
17+
* Resolve the selected download options from the request, auto-detecting the
18+
* operating system from client hints / user agent when not explicitly chosen.
19+
*
20+
* @param array<string, mixed> $get GET parameters (os, osvariant, version, ...)
21+
* @return array<string, mixed>
22+
*/
23+
public function resolve(array $get, string $platformHeader, string $uaHeader): array
24+
{
25+
[$autoOs, $autoOsVariant] = $this->autoDetect($platformHeader, $uaHeader);
26+
27+
$defaults = [
28+
'os' => $autoOs ?? 'linux',
29+
'version' => 'default',
30+
];
31+
32+
$options = array_merge($defaults, $get);
33+
34+
// Ignore an invalid or non-string os parameter (e.g. bots requesting
35+
// ?os=<garbage> or ?os[]=x) so indexing $osList below stays safe.
36+
if (!is_string($options['os']) || !array_key_exists($options['os'], $this->osList)) {
37+
$options['os'] = $defaults['os'];
38+
}
39+
40+
if ($autoOsVariant && (!array_key_exists('osvariant', $options) || !array_key_exists($options['osvariant'], $this->osList[$options['os']]['variants']))) {
41+
$options['osvariant'] = $autoOsVariant;
42+
} elseif (!array_key_exists('osvariant', $options) || !array_key_exists($options['osvariant'], $this->osList[$options['os']]['variants'])) {
43+
$options['osvariant'] = array_key_first($this->osList[$options['os']]['variants']);
44+
}
45+
46+
return $options;
47+
}
48+
49+
/**
50+
* @return array{0: ?string, 1: ?string} [auto os, auto os variant]
51+
*/
52+
private function autoDetect(string $platformHeader, string $uaHeader): array
53+
{
54+
$autoOs = null;
55+
$autoOsVariant = null;
56+
57+
if ($platformHeader === '' && $uaHeader === '') {
58+
return [$autoOs, $autoOsVariant];
59+
}
60+
61+
$platform = strtolower(trim($platformHeader, '"'));
62+
63+
if ($platform === 'windows' || stripos($uaHeader, 'Windows') !== false) {
64+
$autoOs = 'windows';
65+
} elseif ($platform === 'macos' || stripos($uaHeader, 'Mac') !== false) {
66+
$autoOs = 'osx';
67+
} elseif ($platform === 'linux' || stripos($uaHeader, 'Linux') !== false) {
68+
$autoOs = 'linux';
69+
if (stripos($uaHeader, 'Ubuntu') !== false) {
70+
$autoOsVariant = 'linux-ubuntu';
71+
} elseif (stripos($uaHeader, 'Debian') !== false) {
72+
$autoOsVariant = 'linux-debian';
73+
} elseif (stripos($uaHeader, 'Fedora') !== false) {
74+
$autoOsVariant = 'linux-fedora';
75+
} elseif (stripos($uaHeader, 'Red Hat') !== false || stripos($uaHeader, 'RedHat') !== false) {
76+
$autoOsVariant = 'linux-redhat';
77+
}
78+
}
79+
80+
return [$autoOs, $autoOsVariant];
81+
}
82+
}

‎tests/Unit/Downloads/OptionResolverTest.php‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace phpweb\Test\Unit\Downloads;
6+
7+
use phpweb\Downloads\OptionResolver;
8+
use PHPUnit\Framework;
9+
10+
#[Framework\Attributes\CoversClass(OptionResolver::class)]
11+
class OptionResolverTest extends Framework\TestCase
12+
{
13+
/**
14+
* @var array<string, array{name: string, variants: array<string, string>}>
15+
*/
16+
private const OS_LIST = [
17+
'linux' => [
18+
'name' => 'Linux',
19+
'variants' => [
20+
'linux-debian' => 'Debian',
21+
'linux-ubuntu' => 'Ubuntu',
22+
],
23+
],
24+
'osx' => [
25+
'name' => 'macOS',
26+
'variants' => [
27+
'osx-homebrew' => 'Homebrew',
28+
'osx-macports' => 'MacPorts',
29+
],
30+
],
31+
'windows' => [
32+
'name' => 'Windows',
33+
'variants' => [
34+
'windows-downloads' => 'ZIP Downloads',
35+
'windows-native' => 'Single Line Installer',
36+
],
37+
],
38+
];
39+
40+
/**
41+
* Reproduces the production crash: a bot requesting downloads.php?os=<garbage>
42+
* previously made $os[$options['os']] null and threw
43+
* "array_key_exists(): Argument #2 ($array) must be of type array, null given".
44+
*/
45+
public function testInvalidOsParameterFallsBackToDefault(): void
46+
{
47+
$options = $this->resolver()->resolve(['os' => 'not-a-real-os'], '', '');
48+
49+
self::assertSame('linux', $options['os']);
50+
self::assertSame('linux-debian', $options['osvariant']);
51+
}
52+
53+
public function testArrayOsParameterFallsBackToDefault(): void
54+
{
55+
$options = $this->resolver()->resolve(['os' => ['linux']], '', '');
56+
57+
self::assertSame('linux', $options['os']);
58+
}
59+
60+
public function testValidOsParameterSelectsFirstVariant(): void
61+
{
62+
$options = $this->resolver()->resolve(['os' => 'windows'], '', '');
63+
64+
self::assertSame('windows', $options['os']);
65+
self::assertSame('windows-downloads', $options['osvariant']);
66+
}
67+
68+
public function testValidOsAndVariantAreHonored(): void
69+
{
70+
$options = $this->resolver()->resolve(
71+
['os' => 'osx', 'osvariant' => 'osx-macports'],
72+
'',
73+
'',
74+
);
75+
76+
self::assertSame('osx', $options['os']);
77+
self::assertSame('osx-macports', $options['osvariant']);
78+
}
79+
80+
public function testInvalidVariantFallsBackToFirstForThatOs(): void
81+
{
82+
$options = $this->resolver()->resolve(
83+
['os' => 'windows', 'osvariant' => 'linux-debian'],
84+
'',
85+
'',
86+
);
87+
88+
self::assertSame('windows', $options['os']);
89+
self::assertSame('windows-downloads', $options['osvariant']);
90+
}
91+
92+
public function testVersionDefaultsWhenNotSupplied(): void
93+
{
94+
$options = $this->resolver()->resolve([], '', '');
95+
96+
self::assertSame('default', $options['version']);
97+
}
98+
99+
public function testGetParametersArePreserved(): void
100+
{
101+
$options = $this->resolver()->resolve(
102+
['os' => 'linux', 'version' => '8.4', 'source' => 'Y'],
103+
'',
104+
'',
105+
);
106+
107+
self::assertSame('8.4', $options['version']);
108+
self::assertSame('Y', $options['source']);
109+
}
110+
111+
public function testDetectsWindowsFromUserAgent(): void
112+
{
113+
$options = $this->resolver()->resolve([], '', 'Mozilla/5.0 (Windows NT 10.0; Win64; x64)');
114+
115+
self::assertSame('windows', $options['os']);
116+
}
117+
118+
public function testDetectsUbuntuVariantFromUserAgent(): void
119+
{
120+
$options = $this->resolver()->resolve([], '', 'Mozilla/5.0 (X11; Ubuntu; Linux x86_64)');
121+
122+
self::assertSame('linux', $options['os']);
123+
self::assertSame('linux-ubuntu', $options['osvariant']);
124+
}
125+
126+
public function testExplicitOsParameterOverridesAutoDetection(): void
127+
{
128+
$options = $this->resolver()->resolve(
129+
['os' => 'osx'],
130+
'',
131+
'Mozilla/5.0 (Windows NT 10.0; Win64; x64)',
132+
);
133+
134+
self::assertSame('osx', $options['os']);
135+
}
136+
137+
private function resolver(): OptionResolver
138+
{
139+
return new OptionResolver(self::OS_LIST);
140+
}
141+
}

0 commit comments

Comments
 (0)