Fix GH-17797: zend_test_compile_string crash on invalid script path. #17801
Conversation
main/fopen_wrappers.c
Outdated
| @@ -603,7 +603,9 @@ PHPAPI zend_string *php_resolve_path(const char *filename, size_t filename_lengt | |||
| const char *exec_fname = ZSTR_VAL(exec_filename); | |||
| size_t exec_fname_length = ZSTR_LEN(exec_filename); | |||
|
|
|||
| while ((--exec_fname_length < SIZE_MAX) && !IS_SLASH(exec_fname[exec_fname_length])); | |||
| do { | |||
There was a problem hiding this comment.
Can't this loop be rewritten in a cleaner way?
Like: check that the var is not 0 in the while loop condition along with the IS_SLASH, and modify the length in the while loop body. That seems more readable and cleaner. WDYT?
There was a problem hiding this comment.
matter of taste I would say, it does not change much things as readability IMHO but don t really object.
There was a problem hiding this comment.
Sure. But please at least put {} around the break (and break on a new line), as now the code doesn't really comply with our code style standard.
There was a problem hiding this comment.
Something broke on Windows. And it's because the macro evaluates the expression c twice on Windows:
php-src/Zend/zend_virtual_cwd.h
Line 77 in b3dd5a4
If you follow my suggestion of https://github.com/php/php-src/pull/17801/files#r1956774278 then you can resolve this
ahah good to know. |
main/fopen_wrappers.c
Outdated
| @@ -603,7 +603,10 @@ PHPAPI zend_string *php_resolve_path(const char *filename, size_t filename_lengt | |||
| const char *exec_fname = ZSTR_VAL(exec_filename); | |||
| size_t exec_fname_length = ZSTR_LEN(exec_filename); | |||
|
|
|||
| while ((--exec_fname_length < SIZE_MAX) && !IS_SLASH(exec_fname[exec_fname_length])); | |||
| while (exec_fname_length != 0 && !IS_SLASH(exec_fname[exec_fname_length])) { | |||
There was a problem hiding this comment.
Shouldn't this be:
| while (exec_fname_length != 0 && !IS_SLASH(exec_fname[exec_fname_length])) { | |
| while (exec_fname_length != 0 && !IS_SLASH(exec_fname[exec_fname_length - 1])) { |
or am I missing something?
devnexen
force-pushed
the
gh17797
branch
3 times, most recently
from
6b5cd60 to
7e3283a
Compare
When looking for the last slash of the script path, it leads to underflow being promoted to SIZE_MAX being way beyond MAXPATHLEN. close phpGH-17801
When looking for the last slash of the script path, it leads to underflow being promoted to SIZE_MAX being way beyond MAXPATHLEN.