From 5b5dc4a85bde6e959a0943ac694833784a838164 Mon Sep 17 00:00:00 2001
From: Jeffrey Altman <jaltman@secure-endpoints.com>
Date: Mon, 14 Sep 2009 07:59:54 -0700
Subject: [PATCH] Windows: more pioctl output validation

Add output validation checks to the Explorer Shell and the
Client configuration control panel.

LICENSE MIT

Reviewed-on: http://gerrit.openafs.org/458
Reviewed-by: Jeffrey Altman <jaltman@openafs.org>
Tested-by: Jeffrey Altman <jaltman@openafs.org>
---
 src/WINNT/client_config/config.cpp   | 16 +++++++++-------
 src/WINNT/client_config/drivemap.cpp | 15 ++++++++++++---
 src/WINNT/client_exp/gui2fs.cpp      | 17 ++++++++++++-----
 3 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/src/WINNT/client_config/config.cpp b/src/WINNT/client_config/config.cpp
index f5be8916d..b576d72ae 100644
--- a/src/WINNT/client_config/config.cpp
+++ b/src/WINNT/client_config/config.cpp
@@ -172,6 +172,7 @@ void Config_GetCellName (LPTSTR pszName)
 
 BOOL Config_ContactGateway (LPTSTR pszGateway, LPTSTR pszCell)
 {
+   // pszCell is MAX_PATH
    BOOL rc = FALSE;
 
    BYTE OutData[ PIOCTL_MAXSIZE ];
@@ -189,13 +190,14 @@ BOOL Config_ContactGateway (LPTSTR pszGateway, LPTSTR pszCell)
 
    ULONG status;
    if ((status = pioctl (0, VIOC_GET_WS_CELL, &IOInfo, 1)) == 0)
-      {
-      if (OutData[0])
-         {
-         lstrcpy (pszCell, (LPCTSTR)OutData);
-         rc = TRUE;
-         }
-      }
+   {
+       OutData[min(PIOCTL_MAXSIZE, MAX_PATH) - 1] = '\0';
+       if (OutData[0])
+       {
+           lstrcpy (pszCell, (LPCTSTR)OutData);
+           rc = TRUE;
+       }
+   }
 
    Config_SetGatewayName (szOldGateway);
 
diff --git a/src/WINNT/client_config/drivemap.cpp b/src/WINNT/client_config/drivemap.cpp
index 5485950a1..4ae9087e3 100644
--- a/src/WINNT/client_config/drivemap.cpp
+++ b/src/WINNT/client_config/drivemap.cpp
@@ -786,8 +786,11 @@ void FreeDriveMapList (PDRIVEMAPLIST pList)
 }
 
 
+
 BOOL PathToSubmount (LPTSTR pszSubmount, LPTSTR pszMapping, LPTSTR pszSubmountReq, ULONG *pStatus)
 {
+   // pszSubmount is MAX_PATH in length
+
    if (pszSubmountReq && !IsValidSubmountName (pszSubmountReq))
       pszSubmountReq = NULL;
 
@@ -824,6 +827,8 @@ BOOL PathToSubmount (LPTSTR pszSubmount, LPTSTR pszMapping, LPTSTR pszSubmountRe
    if (status)
       return FALSE;
 
+
+   OutData[min(PIOCTL_MAXSIZE, MAX_PATH) - 1] = '\0';
    lstrcpy (pszSubmount, (LPCTSTR)OutData);
    return (pszSubmount[0] != TEXT('\0')) ? TRUE : FALSE;
 }
@@ -1424,9 +1429,12 @@ BOOL GlobalMountDrive()
 
 DWORD MountDOSDrive(char chDrive,const char *szSubmount,BOOL bPersistent,const char * pUsername)
 {
+#ifdef AFSIFS
     DWORD err;
-	BOOL succ;
-	TCHAR szPath[MAX_PATH], szTokens[MAX_PATH], *tok;
+    BOOL succ;
+    TCHAR szTokens[MAX_PATH], *tok;
+#endif /* AFSIFS */
+    TCHAR szPath[MAX_PATH];
     TCHAR szClient[MAX_PATH];
     TCHAR szDrive[3] = TEXT("?:");
 
@@ -1504,8 +1512,9 @@ DWORD DisMountDOSDrive(const char *pSubmount,BOOL bForce)
 DWORD DisMountDOSDrive(const char chDrive,BOOL bForce)
 {
     TCHAR szPath[MAX_PATH];
+#ifdef AFSIFS
     DWORD succ;
-
+#endif
     sprintf(szPath,"%c:",chDrive);
 #ifdef AFSIFS
     succ = DefineDosDevice(DDD_REMOVE_DEFINITION, szPath, NULL);
diff --git a/src/WINNT/client_exp/gui2fs.cpp b/src/WINNT/client_exp/gui2fs.cpp
index de1fbb4ae..c5be1c0a0 100644
--- a/src/WINNT/client_exp/gui2fs.cpp
+++ b/src/WINNT/client_exp/gui2fs.cpp
@@ -284,6 +284,7 @@ void WhichCell(CStringArray& files)
             } else
                 results.Add(GetAfsError(errno));
         } else {
+            space[MAXSIZE - 1] = '\0';
             results.Add(Utf8ToCString(space));
         }
     }       
@@ -1488,7 +1489,9 @@ BOOL ListMount(CStringArray& files)
         last_component.ReleaseBuffer();
 
         if (code == 0) {
-            int nPos = strlen(space) - 1;
+            int nPos;
+            space[MAXSIZE - 1] = '\0';
+            nPos = strlen(space) - 1;
             if (space[nPos] == '.')
                 space[nPos] = 0;
             mountPoints.Add(ParseMountPoint(StripPath(files[i]), Utf8ToCString(space)));
@@ -1796,7 +1799,7 @@ BOOL GetVolumeInfo(CString strFile, CVolInfo& volInfo)
     blob.out = space;
 
     code = pioctl_T(strFile, VIOCGETVOLSTAT, &blob, 1);
-    if (code) {
+    if (code || blob.out_size < sizeof(*status)) {
         volInfo.m_strErrorMsg = GetAfsError(errno, strFile);
         return FALSE;
     }
@@ -1855,7 +1858,7 @@ BOOL SetVolInfo(CVolInfo& volInfo)
 #endif
 
     code = pioctl_T(volInfo.m_strFilePath, VIOCSETVOLSTAT, &blob, 1);
-    if (code) {
+    if (code || blob.out_size < sizeof(*status)) {
         ShowMessageBox(IDS_SET_QUOTA_ERROR, MB_ICONERROR, IDS_SET_QUOTA_ERROR, GetAfsError(errno, volInfo.m_strName));
         return FALSE;
     }
@@ -2169,8 +2172,12 @@ BOOL ListSymlink(CStringArray& files)
         ustrLast.ReleaseBuffer();
 
         if (code == 0) {
-            CString syml = Utf8ToCString(space);
-            int len = syml.GetLength();
+            CString syml;
+            int len;
+
+            space[MAXSIZE - 1] = '\0';
+            syml = Utf8ToCString(space);
+            len = syml.GetLength();
 
             if (len > 0) {
                 if (syml[len - 1] == _T('.'))
-- 
2.39.5