Skip to content

ext/soap: Add test for Set-Cookie option parsing bug. #21400

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
devnexen wants to merge 3 commits into from
Closed

ext/soap: Add test for Set-Cookie option parsing bug. #21400

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7477efc
ext/soap: Add test for Set-Cookie option parsing bug.
devnexen Mar 9, 2026
a1dd4ba
- remove unneeded shadowed var.
devnexen Mar 9, 2026
f3698fa
review
devnexen Mar 10, 2026
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
3 changes: 1 addition & 2 deletions ext/soap/php_http.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1010,7 +1010,6 @@ 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
Copy Markdown
Member

@ndossche ndossche Mar 10, 2026

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;

if (sempos != NULL) {
Expand All @@ -1026,7 +1025,7 @@ int make_http_soap_request(zval *this_ptr,
add_index_stringl(&zcookie, 0, eqpos + 1, cookie_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
View file
Open in desktop
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