From: Anatol Belski <ab@php.net>
Date: Tue, 19 May 2015 13:44:55 +0000 (+0200)
Subject: Fixed bug #69511 Off-by-one bufferoverflow in php_sys_readlink
X-Git-Tag: PRE_PHP7_NSAPI_REMOVAL~42^2~38
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=890a28d4b97b5785f155618fc34134acb77f7a64;p=php

Fixed bug #69511 Off-by-one bufferoverflow in php_sys_readlink
---

diff --git a/NEWS b/NEWS
index 2a3019a4c9..7ca0e33748 100644
--- a/NEWS
+++ b/NEWS
@@ -78,6 +78,8 @@
   . Implemented the RFC `Fix "foreach" behavior`. (Dmitry)
   . Implemented the RFC `Generator Delegation`. (Bob)
   . Implemented the RFC ` Anonymous Class Support`. (Joe, Nikita, Dmitry)
+  . Fixed bug #69511 (Off-by-one buffer overflow in php_sys_readlink).
+    (Jan Starke, Anatol)
 
 - Curl:
   . Fixed bug #68937 (Segfault in curl_multi_exec). (Laruence)
diff --git a/Zend/zend_virtual_cwd.c b/Zend/zend_virtual_cwd.c
index 28cc7f6eed..e880775ea7 100644
--- a/Zend/zend_virtual_cwd.c
+++ b/Zend/zend_virtual_cwd.c
@@ -237,6 +237,10 @@ CWD_API int php_sys_readlink(const char *link, char *target, size_t target_len){
 	typedef BOOL (WINAPI *gfpnh_func)(HANDLE, LPTSTR, DWORD, DWORD);
 	gfpnh_func pGetFinalPathNameByHandle;
 
+	if (!target_len) {
+		return -1;
+	}
+
 	kernel32 = LoadLibrary("kernel32.dll");
 
 	if (kernel32) {
@@ -260,8 +264,14 @@ CWD_API int php_sys_readlink(const char *link, char *target, size_t target_len){
 			return -1;
 	}
 
-	dwRet = pGetFinalPathNameByHandle(hFile, target, MAXPATHLEN, VOLUME_NAME_DOS);
-	if(dwRet >= MAXPATHLEN || dwRet == 0) {
+	/* Despite MSDN has documented it won't to, the length returned by
+		GetFinalPathNameByHandleA includes the length of the
+		null terminator. This behavior is at least reproducible
+		with VS2012 and earlier, and seems not to be fixed till
+		now. Thus, correcting target_len so it's suddenly don't
+		overflown. */
+	dwRet = pGetFinalPathNameByHandle(hFile, target, target_len - 1, VOLUME_NAME_DOS);
+	if(dwRet >= target_len || dwRet >= MAXPATHLEN || dwRet == 0) {
 		return -1;
 	}