From ab5e6280115c582ba929ec01055b90b1a22cf8db Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 7 Dec 2024 23:32:49 +0100 Subject: [PATCH 1/2] Fix bug #79075: FFI header parser chokes on comments The directives for FFI should be first in the file, which is fine, however sometimes there can be comments or whitespace before or between these defines. One practical example is for license information or when a user adds newlines "by accident". In these cases, it's quite confusing that the directives do not work properly. To solve this, make the zend_ffi_parse_directives() aware of comments. --- ext/ffi/ffi.c | 122 +++++++++++++++++------------------- ext/ffi/tests/bug79075.h | 12 ++++ ext/ffi/tests/bug79075.inc | 3 + ext/ffi/tests/bug79075.phpt | 25 ++++++++ 4 files changed, 96 insertions(+), 66 deletions(-) create mode 100644 ext/ffi/tests/bug79075.h create mode 100644 ext/ffi/tests/bug79075.inc create mode 100644 ext/ffi/tests/bug79075.phpt diff --git a/ext/ffi/ffi.c b/ext/ffi/ffi.c index 06a79d250a633..eade32619b8c3 100644 --- a/ext/ffi/ffi.c +++ b/ext/ffi/ffi.c @@ -4958,85 +4958,80 @@ ZEND_METHOD(FFI_CType, getFuncParameterType) /* {{{ */ } /* }}} */ +static char *zend_ffi_skip_ws_and_comments(char *p, bool allow_standalone_newline) +{ + while (true) { + if (*p == ' ' || *p == '\t') { + p++; + } else if (allow_standalone_newline && (*p == '\r' || *p == '\n' || *p == '\f' || *p == '\v')) { + p++; + } else if (allow_standalone_newline && *p == '/' && p[1] == '/') { + p += 2; + while (*p && *p != '\r' && *p != '\n') { + p++; + } + } else if (*p == '/' && p[1] == '*') { + p += 2; + while (*p && (*p != '*' || p[1] != '/')) { + p++; + } + p += 2; + } else { + break; + } + } + + return p; +} + static char *zend_ffi_parse_directives(const char *filename, char *code_pos, char **scope_name, char **lib, bool preload) /* {{{ */ { char *p; + code_pos = zend_ffi_skip_ws_and_comments(code_pos, true); + *scope_name = NULL; *lib = NULL; while (*code_pos == '#') { - if (strncmp(code_pos, "#define FFI_SCOPE", sizeof("#define FFI_SCOPE") - 1) == 0 - && (code_pos[sizeof("#define FFI_SCOPE") - 1] == ' ' - || code_pos[sizeof("#define FFI_SCOPE") - 1] == '\t')) { - p = code_pos + sizeof("#define FFI_SCOPE"); - while (*p == ' ' || *p == '\t') { - p++; - } - if (*p != '"') { - if (preload) { - zend_error(E_WARNING, "FFI: failed pre-loading '%s', bad FFI_SCOPE define", filename); - } else { - zend_throw_error(zend_ffi_exception_ce, "Failed loading '%s', bad FFI_SCOPE define", filename); - } - return NULL; - } - p++; - if (*scope_name) { - if (preload) { - zend_error(E_WARNING, "FFI: failed pre-loading '%s', FFI_SCOPE defined twice", filename); - } else { - zend_throw_error(zend_ffi_exception_ce, "Failed loading '%s', FFI_SCOPE defined twice", filename); - } - return NULL; - } - *scope_name = p; - while (1) { - if (*p == '\"') { - *p = 0; + if (strncmp(code_pos, ZEND_STRL("#define")) == 0) { + p = zend_ffi_skip_ws_and_comments(code_pos + sizeof("#define") - 1, false); + + char **target = NULL; + const char *target_name = NULL; + if (strncmp(p, ZEND_STRL("FFI_SCOPE")) == 0) { + p = zend_ffi_skip_ws_and_comments(p + sizeof("FFI_SCOPE") - 1, false); + target = scope_name; + target_name = "FFI_SCOPE"; + } else if (strncmp(p, ZEND_STRL("FFI_LIB")) == 0) { + p = zend_ffi_skip_ws_and_comments(p + sizeof("FFI_LIB") - 1, false); + target = lib; + target_name = "FFI_LIB"; + } else { + while (*p && *p != '\n' && *p != '\r') { p++; - break; - } else if (*p <= ' ') { - if (preload) { - zend_error(E_WARNING, "FFI: failed pre-loading '%s', bad FFI_SCOPE define", filename); - } else { - zend_throw_error(zend_ffi_exception_ce, "Failed loading '%s', bad FFI_SCOPE define", filename); - } - return NULL; } - p++; - } - while (*p == ' ' || *p == '\t') { - p++; - } - while (*p == '\r' || *p == '\n') { - p++; - } - code_pos = p; - } else if (strncmp(code_pos, "#define FFI_LIB", sizeof("#define FFI_LIB") - 1) == 0 - && (code_pos[sizeof("#define FFI_LIB") - 1] == ' ' - || code_pos[sizeof("#define FFI_LIB") - 1] == '\t')) { - p = code_pos + sizeof("#define FFI_LIB"); - while (*p == ' ' || *p == '\t') { - p++; + code_pos = zend_ffi_skip_ws_and_comments(p, true); + continue; } + if (*p != '"') { if (preload) { - zend_error(E_WARNING, "FFI: failed pre-loading '%s', bad FFI_LIB define", filename); + zend_error(E_WARNING, "FFI: failed pre-loading '%s', bad %s define", filename, target_name); } else { - zend_throw_error(zend_ffi_exception_ce, "Failed loading '%s', bad FFI_LIB define", filename); + zend_throw_error(zend_ffi_exception_ce, "Failed loading '%s', bad %s define", filename, target_name); } return NULL; } p++; - if (*lib) { + if (*target) { if (preload) { - zend_error(E_WARNING, "FFI: failed pre-loading '%s', FFI_LIB defined twice", filename); + zend_error(E_WARNING, "FFI: failed pre-loading '%s', %s defined twice", filename, target_name); } else { - zend_throw_error(zend_ffi_exception_ce, "Failed loading '%s', FFI_LIB defined twice", filename); + zend_throw_error(zend_ffi_exception_ce, "Failed loading '%s', %s defined twice", filename, target_name); } return NULL; } - *lib = p; + *target = p; while (1) { if (*p == '\"') { *p = 0; @@ -5044,21 +5039,16 @@ static char *zend_ffi_parse_directives(const char *filename, char *code_pos, cha break; } else if (*p <= ' ') { if (preload) { - zend_error(E_WARNING, "FFI: failed pre-loading '%s', bad FFI_LIB define", filename); + zend_error(E_WARNING, "FFI: failed pre-loading '%s', bad %s define", filename, target_name); } else { - zend_throw_error(zend_ffi_exception_ce, "Failed loading '%s', bad FFI_LIB define", filename); + zend_throw_error(zend_ffi_exception_ce, "Failed loading '%s', bad %s define", filename, target_name); } return NULL; } p++; } - while (*p == ' ' || *p == '\t') { - p++; - } - while (*p == '\r' || *p == '\n') { - p++; - } - code_pos = p; + + code_pos = zend_ffi_skip_ws_and_comments(p, true); } else { break; } diff --git a/ext/ffi/tests/bug79075.h b/ext/ffi/tests/bug79075.h new file mode 100644 index 0000000000000..22fa6067a347a --- /dev/null +++ b/ext/ffi/tests/bug79075.h @@ -0,0 +1,12 @@ +/* + * Multiline comment + */ + // whitespace line + +#define ignore_this_line 1 + // +#define/* inline */FFI_SCOPE /* multi- +line */ "bug79075" /* end +*/ + +int printf(const char *format, ...); diff --git a/ext/ffi/tests/bug79075.inc b/ext/ffi/tests/bug79075.inc new file mode 100644 index 0000000000000..ab3daa93d4de3 --- /dev/null +++ b/ext/ffi/tests/bug79075.inc @@ -0,0 +1,3 @@ + +--INI-- +ffi.enable=1 +opcache.enable=1 +opcache.enable_cli=1 +opcache.optimization_level=-1 +opcache.preload={PWD}/bug79075.inc +opcache.file_cache_only=0 +--FILE-- +printf("Hello World from %s!\n", "PHP"); +?> +--EXPECT-- +Hello World from PHP! From cdfeebe629ebbc2fc78539baca0a696bbc2a0169 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 9 Dec 2024 20:53:27 +0100 Subject: [PATCH 2/2] Fix potential OOB --- ext/ffi/ffi.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ext/ffi/ffi.c b/ext/ffi/ffi.c index eade32619b8c3..2000e15c29ea1 100644 --- a/ext/ffi/ffi.c +++ b/ext/ffi/ffi.c @@ -4975,7 +4975,12 @@ static char *zend_ffi_skip_ws_and_comments(char *p, bool allow_standalone_newlin while (*p && (*p != '*' || p[1] != '/')) { p++; } - p += 2; + if (*p == '*') { + p++; + if (*p == '/') { + p++; + } + } } else { break; }