Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions ext/soap/php_http.c
Original file line number Diff line number Diff line change
Expand Up @@ -1010,23 +1010,23 @@ int make_http_soap_request(zval *this_ptr,
char *sempos = strstr(cookie, ";");
if (eqpos != NULL && (sempos == NULL || sempos > eqpos)) {
smart_str name = {0};
int cookie_len;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this shadowed var is wrong: it will cause the loop advancement at line 1065 to be wrong: that line is supposed to skip the entire header value. Perhaps this shadowed variable should be renamed cookie_value_len.

zval zcookie;
size_t cookie_value_len;

if (sempos != NULL) {
cookie_len = sempos-(eqpos+1);
cookie_value_len = sempos-(eqpos+1);
} else {
cookie_len = strlen(cookie)-(eqpos-cookie)-1;
cookie_value_len = strlen(cookie)-(eqpos-cookie)-1;
}

smart_str_appendl(&name, cookie, eqpos - cookie);
smart_str_0(&name);

array_init(&zcookie);
add_index_stringl(&zcookie, 0, eqpos + 1, cookie_len);
add_index_stringl(&zcookie, 0, eqpos + 1, cookie_value_len);

if (sempos != NULL) {
char *options = cookie + cookie_len+1;
char *options = sempos + 1;
while (*options) {
while (*options == ' ') {options++;}
sempos = strstr(options, ";");
Expand Down
61 changes: 61 additions & 0 deletions ext/soap/tests/bugs/cookie_parse_options_offset.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
--TEST--
SOAP Set-Cookie option parsing starts at wrong offset due to variable shadowing
--EXTENSIONS--
soap
--SKIPIF--
<?php
if (!file_exists(__DIR__ . "/../../../../sapi/cli/tests/php_cli_server.inc")) {
echo "skip sapi/cli/tests/php_cli_server.inc required but not found";
}
?>
--FILE--
<?php

include __DIR__ . "/../../../../sapi/cli/tests/php_cli_server.inc";

$args = ["-d", "extension_dir=" . ini_get("extension_dir"), "-d", "extension=" . (substr(PHP_OS, 0, 3) == "WIN" ? "php_" : "") . "soap." . PHP_SHLIB_SUFFIX];
if (php_ini_loaded_file()) {
$args[] = "-c";
$args[] = php_ini_loaded_file();
}

// A 10-char name makes the wrong offset land exactly on the value "path=/evil",
// falsely matching it as a path attribute.
$code = <<<'PHP'
header("Content-Type: text/xml");
header("Set-Cookie: sessionkey=path=/evil;domain=good.com");
echo <<<XML
<?xml version="1.0" encoding="UTF-8"?>
<SOAP-ENV:Envelope xmlns:SOAP-ENV="http://schemas.xmlsoap.org/soap/envelope/" xmlns:ns1="test-uri">
<SOAP-ENV:Body>
<ns1:testResponse/>
</SOAP-ENV:Body>
</SOAP-ENV:Envelope>
XML;
PHP;

php_cli_server_start($code, null, $args);

$client = new SoapClient(null, [
'location' => 'http://' . PHP_CLI_SERVER_ADDRESS . '/test/endpoint',
'uri' => 'test-uri',
'trace' => true,
]);

try {
$client->__soapCall("test", []);
} catch (SoapFault $e) {
// Response parsing may fault, cookies are still stored
}

$cookies = $client->__getCookies();

// path should default to "/test" from the request URI, not "/evil" from the value.
echo "value: " . $cookies['sessionkey'][0] . "\n";
echo "path: " . $cookies['sessionkey'][1] . "\n";
echo "domain: " . $cookies['sessionkey'][2] . "\n";
?>
--EXPECT--
value: path=/evil
path: /test
domain: good.com
Loading