Skip to content

Commit ec0bdad

Browse files
nielsdoscharmitro
nielsdos
authored and
charmitro
committed
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. Closes phpGH-17082.
1 parent fb7715c commit ec0bdad

File tree

5 files changed

+104
-66
lines changed

5 files changed

+104
-66
lines changed

‎NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ PHP NEWS
55
- DBA:
66
. Skip test if inifile is disabled. (orlitzky)
77

8+
- FFI:
9+
. Fixed bug #79075 (FFI header parser chokes on comments). (nielsdos)
10+
811
- Iconv:
912
. Fixed bug GH-17047 (UAF on iconv filter failure). (nielsdos)
1013

‎ext/ffi/ffi.c

Lines changed: 61 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -4960,107 +4960,102 @@ ZEND_METHOD(FFI_CType, getFuncParameterType) /* {{{ */
49604960
}
49614961
/* }}} */
49624962

4963+
static char *zend_ffi_skip_ws_and_comments(char *p, bool allow_standalone_newline)
4964+
{
4965+
while (true) {
4966+
if (*p == ' ' || *p == '\t') {
4967+
p++;
4968+
} else if (allow_standalone_newline && (*p == '\r' || *p == '\n' || *p == '\f' || *p == '\v')) {
4969+
p++;
4970+
} else if (allow_standalone_newline && *p == '/' && p[1] == '/') {
4971+
p += 2;
4972+
while (*p && *p != '\r' && *p != '\n') {
4973+
p++;
4974+
}
4975+
} else if (*p == '/' && p[1] == '*') {
4976+
p += 2;
4977+
while (*p && (*p != '*' || p[1] != '/')) {
4978+
p++;
4979+
}
4980+
if (*p == '*') {
4981+
p++;
4982+
if (*p == '/') {
4983+
p++;
4984+
}
4985+
}
4986+
} else {
4987+
break;
4988+
}
4989+
}
4990+
4991+
return p;
4992+
}
4993+
49634994
static char *zend_ffi_parse_directives(const char *filename, char *code_pos, char **scope_name, char **lib, bool preload) /* {{{ */
49644995
{
49654996
char *p;
49664997

4998+
code_pos = zend_ffi_skip_ws_and_comments(code_pos, true);
4999+
49675000
*scope_name = NULL;
49685001
*lib = NULL;
49695002
while (*code_pos == '#') {
4970-
if (strncmp(code_pos, "#define FFI_SCOPE", sizeof("#define FFI_SCOPE") - 1) == 0
4971-
&& (code_pos[sizeof("#define FFI_SCOPE") - 1] == ' '
4972-
|| code_pos[sizeof("#define FFI_SCOPE") - 1] == '\t')) {
4973-
p = code_pos + sizeof("#define FFI_SCOPE");
4974-
while (*p == ' ' || *p == '\t') {
4975-
p++;
4976-
}
4977-
if (*p != '"') {
4978-
if (preload) {
4979-
zend_error(E_WARNING, "FFI: failed pre-loading '%s', bad FFI_SCOPE define", filename);
4980-
} else {
4981-
zend_throw_error(zend_ffi_exception_ce, "Failed loading '%s', bad FFI_SCOPE define", filename);
4982-
}
4983-
return NULL;
4984-
}
4985-
p++;
4986-
if (*scope_name) {
4987-
if (preload) {
4988-
zend_error(E_WARNING, "FFI: failed pre-loading '%s', FFI_SCOPE defined twice", filename);
4989-
} else {
4990-
zend_throw_error(zend_ffi_exception_ce, "Failed loading '%s', FFI_SCOPE defined twice", filename);
4991-
}
4992-
return NULL;
4993-
}
4994-
*scope_name = p;
4995-
while (1) {
4996-
if (*p == '\"') {
4997-
*p = 0;
5003+
if (strncmp(code_pos, ZEND_STRL("#define")) == 0) {
5004+
p = zend_ffi_skip_ws_and_comments(code_pos + sizeof("#define") - 1, false);
5005+
5006+
char **target = NULL;
5007+
const char *target_name = NULL;
5008+
if (strncmp(p, ZEND_STRL("FFI_SCOPE")) == 0) {
5009+
p = zend_ffi_skip_ws_and_comments(p + sizeof("FFI_SCOPE") - 1, false);
5010+
target = scope_name;
5011+
target_name = "FFI_SCOPE";
5012+
} else if (strncmp(p, ZEND_STRL("FFI_LIB")) == 0) {
5013+
p = zend_ffi_skip_ws_and_comments(p + sizeof("FFI_LIB") - 1, false);
5014+
target = lib;
5015+
target_name = "FFI_LIB";
5016+
} else {
5017+
while (*p && *p != '\n' && *p != '\r') {
49985018
p++;
4999-
break;
5000-
} else if (*p <= ' ') {
5001-
if (preload) {
5002-
zend_error(E_WARNING, "FFI: failed pre-loading '%s', bad FFI_SCOPE define", filename);
5003-
} else {
5004-
zend_throw_error(zend_ffi_exception_ce, "Failed loading '%s', bad FFI_SCOPE define", filename);
5005-
}
5006-
return NULL;
50075019
}
5008-
p++;
5009-
}
5010-
while (*p == ' ' || *p == '\t') {
5011-
p++;
5012-
}
5013-
while (*p == '\r' || *p == '\n') {
5014-
p++;
5015-
}
5016-
code_pos = p;
5017-
} else if (strncmp(code_pos, "#define FFI_LIB", sizeof("#define FFI_LIB") - 1) == 0
5018-
&& (code_pos[sizeof("#define FFI_LIB") - 1] == ' '
5019-
|| code_pos[sizeof("#define FFI_LIB") - 1] == '\t')) {
5020-
p = code_pos + sizeof("#define FFI_LIB");
5021-
while (*p == ' ' || *p == '\t') {
5022-
p++;
5020+
code_pos = zend_ffi_skip_ws_and_comments(p, true);
5021+
continue;
50235022
}
5023+
50245024
if (*p != '"') {
50255025
if (preload) {
5026-
zend_error(E_WARNING, "FFI: failed pre-loading '%s', bad FFI_LIB define", filename);
5026+
zend_error(E_WARNING, "FFI: failed pre-loading '%s', bad %s define", filename, target_name);
50275027
} else {
5028-
zend_throw_error(zend_ffi_exception_ce, "Failed loading '%s', bad FFI_LIB define", filename);
5028+
zend_throw_error(zend_ffi_exception_ce, "Failed loading '%s', bad %s define", filename, target_name);
50295029
}
50305030
return NULL;
50315031
}
50325032
p++;
5033-
if (*lib) {
5033+
if (*target) {
50345034
if (preload) {
5035-
zend_error(E_WARNING, "FFI: failed pre-loading '%s', FFI_LIB defined twice", filename);
5035+
zend_error(E_WARNING, "FFI: failed pre-loading '%s', %s defined twice", filename, target_name);
50365036
} else {
5037-
zend_throw_error(zend_ffi_exception_ce, "Failed loading '%s', FFI_LIB defined twice", filename);
5037+
zend_throw_error(zend_ffi_exception_ce, "Failed loading '%s', %s defined twice", filename, target_name);
50385038
}
50395039
return NULL;
50405040
}
5041-
*lib = p;
5041+
*target = p;
50425042
while (1) {
50435043
if (*p == '\"') {
50445044
*p = 0;
50455045
p++;
50465046
break;
50475047
} else if (*p <= ' ') {
50485048
if (preload) {
5049-
zend_error(E_WARNING, "FFI: failed pre-loading '%s', bad FFI_LIB define", filename);
5049+
zend_error(E_WARNING, "FFI: failed pre-loading '%s', bad %s define", filename, target_name);
50505050
} else {
5051-
zend_throw_error(zend_ffi_exception_ce, "Failed loading '%s', bad FFI_LIB define", filename);
5051+
zend_throw_error(zend_ffi_exception_ce, "Failed loading '%s', bad %s define", filename, target_name);
50525052
}
50535053
return NULL;
50545054
}
50555055
p++;
50565056
}
5057-
while (*p == ' ' || *p == '\t') {
5058-
p++;
5059-
}
5060-
while (*p == '\r' || *p == '\n') {
5061-
p++;
5062-
}
5063-
code_pos = p;
5057+
5058+
code_pos = zend_ffi_skip_ws_and_comments(p, true);
50645059
} else {
50655060
break;
50665061
}

‎ext/ffi/tests/bug79075.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/*
2+
* Multiline comment
3+
*/
4+
// whitespace line
5+
6+
#define ignore_this_line 1
7+
//
8+
#define/* inline */FFI_SCOPE /* multi-
9+
line */ "bug79075" /* end
10+
*/
11+
12+
int printf(const char *format, ...);

‎ext/ffi/tests/bug79075.inc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<?php
2+
3+
FFI::load(__DIR__ . "/bug79075.h");

‎ext/ffi/tests/bug79075.phpt

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
--TEST--
2+
Bug #79075 (FFI header parser chokes on comments)
3+
--EXTENSIONS--
4+
ffi
5+
opcache
6+
posix
7+
--SKIPIF--
8+
<?php
9+
if (substr(PHP_OS, 0, 3) == 'WIN') die('skip not for Windows');
10+
if (posix_geteuid() == 0) die('skip Cannot run test as root.');
11+
?>
12+
--INI--
13+
ffi.enable=1
14+
opcache.enable=1
15+
opcache.enable_cli=1
16+
opcache.optimization_level=-1
17+
opcache.preload={PWD}/bug79075.inc
18+
opcache.file_cache_only=0
19+
--FILE--
20+
<?php
21+
$ffi = FFI::scope("bug79075");
22+
$ffi->printf("Hello World from %s!\n", "PHP");
23+
?>
24+
--EXPECT--
25+
Hello World from PHP!

0 commit comments

Comments
 (0)