diff options
| author | Andrew Clayton <a.clayton@nginx.com> | 2022-11-07 00:06:43 +0000 |
|---|---|---|
| committer | Andrew Clayton <a.clayton@nginx.com> | 2023-01-12 17:56:01 +0000 |
| commit | 97caab0e7a40c4034888e3269cdcbb858e47b45b (patch) | |
| tree | 36001a6f6d125f07a91b47467bf01d3578942003 | |
| parent | 2c7e1bb92f6a3091b8fab872b93fa4791fbe14c8 (diff) | |
| download | unit-97caab0e7a40c4034888e3269cdcbb858e47b45b.tar.gz unit-97caab0e7a40c4034888e3269cdcbb858e47b45b.tar.bz2 | |
PHP: Fix a potential problem parsing the path.
@dward on GitHub reported an issue with a URL like
http://foo.bar/test.php?blah=test.php/foo
where we would end up trying to run the script
test.php?blah=test.php
In the PHP module the format 'file.php/' is treated as a special case in
nxt_php_dynamic_request() where we check the _path_ part of the url for
the string '.php/'.
The problem is that the path actually also contains the query string,
thus we were finding 'test.php/' in the above URL and treating that
whole path as the script to run.
The fix is simple, replace the strstr(3) with a memmem(3), where we can
limit the amount of path we use for the check.
The trick here and what is not obvious from the code is that while
path.start points to the whole path including the query string,
path.length only contains the length of the _path_ part.
NOTE: memmem(3) is a GNU extension and is neither specified by POSIX or
ISO C, however it is available on a number of other systems, including:
FreeBSD, OpenBSD, NetBSD, illumos, and macOS.
If it comes to it we can implement a simple alternative for systems
which lack memmem(3).
This also adds a test case (provided by @dward) to cover this.
Closes: <https://github.com/nginx/unit/issues/781>
Cc: Andrei Zeliankou <zelenkov@nginx.com>
Reviewed-by: Alejandro Colomar <alx@nginx.com>
Reviewed-by: Andrei Zeliankou <zelenkov@nginx.com> [test]
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
| -rw-r--r-- | src/nxt_php_sapi.c | 3 | ||||
| -rw-r--r-- | test/test_php_targets.py | 1 |
2 files changed, 3 insertions, 1 deletions
diff --git a/src/nxt_php_sapi.c b/src/nxt_php_sapi.c index 126a4684..d2494938 100644 --- a/src/nxt_php_sapi.c +++ b/src/nxt_php_sapi.c @@ -1025,7 +1025,8 @@ nxt_php_dynamic_request(nxt_php_run_ctx_t *ctx, nxt_unit_request_t *r) nxt_str_null(&script_name); - ctx->path_info.start = (u_char *) strstr((char *) path.start, ".php/"); + ctx->path_info.start = memmem(path.start, path.length, ".php/", + strlen(".php/")); if (ctx->path_info.start != NULL) { ctx->path_info.start += 4; path.length = ctx->path_info.start - path.start; diff --git a/test/test_php_targets.py b/test/test_php_targets.py index 918c5fda..eec1846f 100644 --- a/test/test_php_targets.py +++ b/test/test_php_targets.py @@ -47,6 +47,7 @@ class TestPHPTargets(TestApplicationPHP): assert self.get(url='/2')['body'] == '2' assert self.get(url='/blah')['status'] == 503 # TODO 404 assert self.get(url='/')['body'] == 'index' + assert self.get(url='/1.php?test=test.php/')['body'] == '1' assert 'success' in self.conf( "\"1.php\"", 'applications/targets/targets/default/index' |