Skip to content

Commit 97caab0

Browse files
committed
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: <#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>
1 parent 2c7e1bb commit 97caab0

File tree

2 files changed

+3
-1
lines changed

2 files changed

+3
-1
lines changed

src/nxt_php_sapi.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1025,7 +1025,8 @@ nxt_php_dynamic_request(nxt_php_run_ctx_t *ctx, nxt_unit_request_t *r)
10251025

10261026
nxt_str_null(&script_name);
10271027

1028-
ctx->path_info.start = (u_char *) strstr((char *) path.start, ".php/");
1028+
ctx->path_info.start = memmem(path.start, path.length, ".php/",
1029+
strlen(".php/"));
10291030
if (ctx->path_info.start != NULL) {
10301031
ctx->path_info.start += 4;
10311032
path.length = ctx->path_info.start - path.start;

test/test_php_targets.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ def test_php_application_targets(self):
4747
assert self.get(url='/2')['body'] == '2'
4848
assert self.get(url='/blah')['status'] == 503 # TODO 404
4949
assert self.get(url='/')['body'] == 'index'
50+
assert self.get(url='/1.php?test=test.php/')['body'] == '1'
5051

5152
assert 'success' in self.conf(
5253
"\"1.php\"", 'applications/targets/targets/default/index'

0 commit comments

Comments
 (0)