From 94fa2a4ce103eaa3ac745e2f3d920b81bd60dc18 Mon Sep 17 00:00:00 2001
From: "Christoph M. Becker" <cmbecker69@gmx.de>
Date: Fri, 29 Nov 2024 19:43:07 +0100
Subject: [PATCH] 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 GH-16995.
---
 NEWS                    | 1 +
 Zend/zend_compile.c     | 6 +++---
 Zend/zend_virtual_cwd.h | 5 +++++
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/NEWS b/NEWS
index eca09704cfc..84fdaf00b48 100644
--- a/NEWS
+++ b/NEWS
@@ -19,6 +19,7 @@ PHP                                                                        NEWS
   . Fixed bug GH-16630 (UAF in lexer with encoding translation and heredocs).
     (nielsdos)
   . Fix is_zend_ptr() huge block comparison. (nielsdos)
+  . Fixed potential OOB read in zend_dirname() on Windows. (cmb)
 
 - Curl:
   . Fix various memory leaks in curl mime handling. (nielsdos)
diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
index 38d378a4175..52b3417234c 100644
--- a/Zend/zend_compile.c
+++ b/Zend/zend_compile.c
@@ -1997,7 +1997,7 @@ ZEND_API size_t zend_dirname(char *path, size_t len)
 	}
 
 	/* Strip trailing slashes */
-	while (end >= path && IS_SLASH_P(end)) {
+	while (end >= path && IS_SLASH_P_EX(end, end == path)) {
 		end--;
 	}
 	if (end < path) {
@@ -2008,7 +2008,7 @@ ZEND_API size_t zend_dirname(char *path, size_t len)
 	}
 
 	/* Strip filename */
-	while (end >= path && !IS_SLASH_P(end)) {
+	while (end >= path && !IS_SLASH_P_EX(end, end == path)) {
 		end--;
 	}
 	if (end < path) {
@@ -2019,7 +2019,7 @@ ZEND_API size_t zend_dirname(char *path, size_t len)
 	}
 
 	/* Strip slashes which came before the file name */
-	while (end >= path && IS_SLASH_P(end)) {
+	while (end >= path && IS_SLASH_P_EX(end, end == path)) {
 		end--;
 	}
 	if (end < path) {
diff --git a/Zend/zend_virtual_cwd.h b/Zend/zend_virtual_cwd.h
index 728e3ba69d8..28cbf6300b8 100644
--- a/Zend/zend_virtual_cwd.h
+++ b/Zend/zend_virtual_cwd.h
@@ -73,8 +73,11 @@ typedef unsigned short mode_t;
 #define DEFAULT_SLASH '\\'
 #define DEFAULT_DIR_SEPARATOR	';'
 #define IS_SLASH(c)	((c) == '/' || (c) == '\\')
+// IS_SLASH_P() may read the previous char on Windows, which may be OOB; use IS_SLASH_P_EX() instead
 #define IS_SLASH_P(c)	(*(c) == '/' || \
         (*(c) == '\\' && !IsDBCSLeadByte(*(c-1))))
+#define IS_SLASH_P_EX(c, first_byte)	(*(c) == '/' || \
+        (*(c) == '\\' && ((first_byte) || !IsDBCSLeadByte(*(c-1)))))
 
 /* COPY_WHEN_ABSOLUTE is 2 under Win32 because by chance both regular absolute paths
    in the file system and UNC paths need copying of two characters */
@@ -98,7 +101,9 @@ typedef unsigned short mode_t;
 #endif
 
 #define IS_SLASH(c)	((c) == '/')
+// IS_SLASH_P() may read the previous char on Windows, which may be OOB; use IS_SLASH_P_EX() instead
 #define IS_SLASH_P(c)	(*(c) == '/')
+#define IS_SLASH_P_EX(c, first_byte) IS_SLASH_P(c)
 
 #endif