Skip to content

Fix bug71610.phpt #16063

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
wants to merge 1 commit into from
Closed

Fix bug71610.phpt #16063

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 11 additions & 2 deletions ext/soap/tests/bug71610.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,20 @@ SOAP Bug #71610 - Type Confusion Vulnerability - SOAP / make_http_soap_request()
soap
--SKIPIF--
<?php
if (getenv("SKIP_ONLINE_TESTS")) die("skip online test");
if (!file_exists(__DIR__ . "/../../../sapi/cli/tests/php_cli_server.inc")) {
echo "skip sapi/cli/tests/php_cli_server.inc required but not found";
}
Comment on lines +7 to +9
Copy link
Member

Choose a reason for hiding this comment

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

I think that we should assume this file to always exist, and let the test fail if it does not. Otherwise it will silently become always-skipped if we move the file without updating it.

I would suggest to use include __DIR__ . "/../../../sapi/cli/tests/skipif.inc" instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, for our purposes, at least regarding CI, this is certainly a good idea. However, some may build ext/soap via phpize (I assume distro managers do that), and in that case it is unlikely that the file exists. include will trigger a warning, and the test would be reported as borked.

Given that the file is already used elsewhere, that opcache has its own (diverged) copy, and that I would like to use this server for ext/standard/tests/http and ext/ftp/tests (these currently have servers which require the posix extension), I hope we can come up with a more general solution.

Not perfect, and might have issues, but what about storing symlinks in the repository?

Copy link
Member

Choose a reason for hiding this comment

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

I didn't realize these tests could be executed in a standalone pecl build. Skipping the test like this seems reasonable then.

I agree that it would be nice to have a more general solution at some point.

Copy link
Member

Choose a reason for hiding this comment

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

Not perfect, and might have issues, but what about storing symlinks in the repository?

Do you mean symlinking the .inc file into ext/soap so that exports of the ext source code also contain it? I don't have experience with symlinks in git, but this could be an option indeed. An alternative could be to distribute/install a testing library along with run-tests.php or phpize (with the downside that evolving this library would be subject to BC).

Copy link
Member Author

Choose a reason for hiding this comment

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

I've already did #16066, but @iluuu1994 suggested to go with a simple PHP include file instead of the symlinks (these have issues anyway; you need Windows 10 with developer mode enabled to have them properly work, and a somewhat recent git version, properly configure to accepts symlinks). And I guess, exporting the sources will not automatically resolve the symlinks. We could do that manually, though, through ./configure. (not sure about the details)

An alternative could be to distribute/install a testing library along with run-tests.php or phpize (with the downside that evolving this library would be subject to BC).

Yeah, that is what I would like, but I also see the problems.

Still, a first step to avoid those lengthy "parent paths" would be a good thing[TM].

?>
--FILE--
<?php
$exploit = unserialize('O:10:"SoapClient":3:{s:3:"uri";s:1:"a";s:8:"location";s:19:"http://example.org/";s:8:"_cookies";a:1:{s:8:"manhluat";a:3:{i:0;s:0:"";i:1;N;i:2;N;}}}');
include __DIR__ . "/../../../sapi/cli/tests/php_cli_server.inc";
php_cli_server_start();

$url = "http://" . PHP_CLI_SERVER_ADDRESS;
$ser = 'O:10:"SoapClient":3:{s:3:"uri";s:1:"a";s:8:"location";s:' . strlen($url) . ':"'
. $url . '";s:8:"_cookies";a:1:{s:8:"manhluat";a:3:{i:0;s:0:"";i:1;N;i:2;N;}}}';

$exploit = unserialize($ser);
try {
$exploit->blahblah();
} catch(SoapFault $e) {
Expand Down
Loading