Skip to content

Commit bf7c74a

Browse files
cmb69charmitro
authored andcommitted
Fix potential OOB read in zend_dirname() on Windows
Only on Windows `IS_SLASH_P()` may read the previous byte, and so may in unlikely cases read one byte out of bounds. Since `IS_SLASH_P()` is in a public header (albeit not likely to be used by external extensions or SAPIs), we introduce `IS_SLASH_P_EX()` which accepts a second argument to prevent that OOB read. It should be noted that the PHP userland function `dirname()` is not affected by this issue, since it does not call `zend_dirname()` on Windows. Closes phpGH-16995.
1 parent 91b8ac7 commit bf7c74a

File tree

3 files changed

+9
-3
lines changed

3 files changed

+9
-3
lines changed

NEWS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ PHP NEWS
1919
. Fixed bug GH-16630 (UAF in lexer with encoding translation and heredocs).
2020
(nielsdos)
2121
. Fix is_zend_ptr() huge block comparison. (nielsdos)
22+
. Fixed potential OOB read in zend_dirname() on Windows. (cmb)
2223

2324
- Curl:
2425
. Fix various memory leaks in curl mime handling. (nielsdos)

Zend/zend_compile.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2201,7 +2201,7 @@ ZEND_API size_t zend_dirname(char *path, size_t len)
22012201
}
22022202

22032203
/* Strip trailing slashes */
2204-
while (end >= path && IS_SLASH_P(end)) {
2204+
while (end >= path && IS_SLASH_P_EX(end, end == path)) {
22052205
end--;
22062206
}
22072207
if (end < path) {
@@ -2212,7 +2212,7 @@ ZEND_API size_t zend_dirname(char *path, size_t len)
22122212
}
22132213

22142214
/* Strip filename */
2215-
while (end >= path && !IS_SLASH_P(end)) {
2215+
while (end >= path && !IS_SLASH_P_EX(end, end == path)) {
22162216
end--;
22172217
}
22182218
if (end < path) {
@@ -2223,7 +2223,7 @@ ZEND_API size_t zend_dirname(char *path, size_t len)
22232223
}
22242224

22252225
/* Strip slashes which came before the file name */
2226-
while (end >= path && IS_SLASH_P(end)) {
2226+
while (end >= path && IS_SLASH_P_EX(end, end == path)) {
22272227
end--;
22282228
}
22292229
if (end < path) {

Zend/zend_virtual_cwd.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,11 @@ typedef unsigned short mode_t;
7575
#define DEFAULT_SLASH '\\'
7676
#define DEFAULT_DIR_SEPARATOR ';'
7777
#define IS_SLASH(c) ((c) == '/' || (c) == '\\')
78+
// IS_SLASH_P() may read the previous char on Windows, which may be OOB; use IS_SLASH_P_EX() instead
7879
#define IS_SLASH_P(c) (*(c) == '/' || \
7980
(*(c) == '\\' && !IsDBCSLeadByte(*(c-1))))
81+
#define IS_SLASH_P_EX(c, first_byte) (*(c) == '/' || \
82+
(*(c) == '\\' && ((first_byte) || !IsDBCSLeadByte(*(c-1)))))
8083

8184
/* COPY_WHEN_ABSOLUTE is 2 under Win32 because by chance both regular absolute paths
8285
in the file system and UNC paths need copying of two characters */
@@ -110,7 +113,9 @@ typedef unsigned short mode_t;
110113
#endif
111114

112115
#define IS_SLASH(c) ((c) == '/')
116+
// IS_SLASH_P() may read the previous char on Windows, which may be OOB; use IS_SLASH_P_EX() instead
113117
#define IS_SLASH_P(c) (*(c) == '/')
118+
#define IS_SLASH_P_EX(c, first_byte) IS_SLASH_P(c)
114119

115120
#endif
116121

0 commit comments

Comments
 (0)