Skip to content

Commit 1b61d55

Browse files
committed
ext/soap: Fix wrong cookie options offset calculation, using separator offset instead.
The cookie option parser uses a wrong offset to start scanning attributes, causing cookie values containing substrings like "path=" or "domain=" to be falsely matched as attributes. close GH-21400
1 parent c4c1261 commit 1b61d55

File tree

3 files changed

+70
-5
lines changed

3 files changed

+70
-5
lines changed

NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ PHP NEWS
1717
. Fixed bug GH-21336 (SNMP::setSecurity() undefined behavior with
1818
NULL arguments). (David Carlier)
1919

20+
- SOAP:
21+
. Fixed Set-Cookie parsing bug wrong offset while scanning attributes.
22+
(David Carlier)
23+
2024
- Standard:
2125
. Fixed bug GH-20906 (Assertion failure when messing up output buffers).
2226
(ndossche)

ext/soap/php_http.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1010,23 +1010,23 @@ int make_http_soap_request(zval *this_ptr,
10101010
char *sempos = strstr(cookie, ";");
10111011
if (eqpos != NULL && (sempos == NULL || sempos > eqpos)) {
10121012
smart_str name = {0};
1013-
int cookie_len;
10141013
zval zcookie;
1014+
size_t cookie_value_len;
10151015

10161016
if (sempos != NULL) {
1017-
cookie_len = sempos-(eqpos+1);
1017+
cookie_value_len = sempos-(eqpos+1);
10181018
} else {
1019-
cookie_len = strlen(cookie)-(eqpos-cookie)-1;
1019+
cookie_value_len = strlen(cookie)-(eqpos-cookie)-1;
10201020
}
10211021

10221022
smart_str_appendl(&name, cookie, eqpos - cookie);
10231023
smart_str_0(&name);
10241024

10251025
array_init(&zcookie);
1026-
add_index_stringl(&zcookie, 0, eqpos + 1, cookie_len);
1026+
add_index_stringl(&zcookie, 0, eqpos + 1, cookie_value_len);
10271027

10281028
if (sempos != NULL) {
1029-
char *options = cookie + cookie_len+1;
1029+
char *options = sempos + 1;
10301030
while (*options) {
10311031
while (*options == ' ') {options++;}
10321032
sempos = strstr(options, ";");
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
--TEST--
2+
SOAP Set-Cookie option parsing starts at wrong offset due to variable shadowing
3+
--EXTENSIONS--
4+
soap
5+
--SKIPIF--
6+
<?php
7+
if (!file_exists(__DIR__ . "/../../../../sapi/cli/tests/php_cli_server.inc")) {
8+
echo "skip sapi/cli/tests/php_cli_server.inc required but not found";
9+
}
10+
?>
11+
--FILE--
12+
<?php
13+
14+
include __DIR__ . "/../../../../sapi/cli/tests/php_cli_server.inc";
15+
16+
$args = ["-d", "extension_dir=" . ini_get("extension_dir"), "-d", "extension=" . (substr(PHP_OS, 0, 3) == "WIN" ? "php_" : "") . "soap." . PHP_SHLIB_SUFFIX];
17+
if (php_ini_loaded_file()) {
18+
$args[] = "-c";
19+
$args[] = php_ini_loaded_file();
20+
}
21+
22+
// A 10-char name makes the wrong offset land exactly on the value "path=/evil",
23+
// falsely matching it as a path attribute.
24+
$code = <<<'PHP'
25+
header("Content-Type: text/xml");
26+
header("Set-Cookie: sessionkey=path=/evil;domain=good.com");
27+
echo <<<XML
28+
<?xml version="1.0" encoding="UTF-8"?>
29+
<SOAP-ENV:Envelope xmlns:SOAP-ENV="http://schemas.xmlsoap.org/soap/envelope/" xmlns:ns1="test-uri">
30+
<SOAP-ENV:Body>
31+
<ns1:testResponse/>
32+
</SOAP-ENV:Body>
33+
</SOAP-ENV:Envelope>
34+
XML;
35+
PHP;
36+
37+
php_cli_server_start($code, null, $args);
38+
39+
$client = new SoapClient(null, [
40+
'location' => 'http://' . PHP_CLI_SERVER_ADDRESS . '/test/endpoint',
41+
'uri' => 'test-uri',
42+
'trace' => true,
43+
]);
44+
45+
try {
46+
$client->__soapCall("test", []);
47+
} catch (SoapFault $e) {
48+
// Response parsing may fault, cookies are still stored
49+
}
50+
51+
$cookies = $client->__getCookies();
52+
53+
// path should default to "/test" from the request URI, not "/evil" from the value.
54+
echo "value: " . $cookies['sessionkey'][0] . "\n";
55+
echo "path: " . $cookies['sessionkey'][1] . "\n";
56+
echo "domain: " . $cookies['sessionkey'][2] . "\n";
57+
?>
58+
--EXPECT--
59+
value: path=/evil
60+
path: /test
61+
domain: good.com

0 commit comments

Comments
 (0)