4703. [bug] BINDInstall.exe was missing some buffer length checks.

                        [RT #45898]

diff --git a/CHANGES b/CHANGES
index 409c638ce162c157a34e9a0e59c2ab3272afa1b9..3333d2ad41fec42a1596c19b8dc3b4aaa65d2832 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,6 @@
+4703.  [bug]           BINDInstall.exe was missing some buffer length checks.
+                       [RT #45898]
+
 4702.  [func]          Update function declarations to use
                        dns_masterstyle_flags_t for style flags. [RT #45924]
 

diff --git a/bin/win32/BINDInstall/AccountInfo.cpp b/bin/win32/BINDInstall/AccountInfo.cpp
index 246dd2c09341bb5c011c822187d82c81d4c85e41..678cd7686ae45203dc03795e19c3bb4a74202102 100644 (file)
--- a/bin/win32/BINDInstall/AccountInfo.cpp
+++ b/bin/win32/BINDInstall/AccountInfo.cpp
@@ -106,6 +106,7 @@ GetAccountPrivileges(char *name, wchar_t **PrivList, unsigned int *PrivCount,
        NTSTATUS Status;
        isc_result_t istatus;
        int iRetVal = RTN_ERROR;        /* assume error from main */
+       int n;
 
        /*
         * Open the policy on the target machine.
@@ -118,18 +119,29 @@ GetAccountPrivileges(char *name, wchar_t **PrivList, unsigned int *PrivCount,
        /*
         * Let's see if the account exists. Return if not
         */
-       wsprintf(AccountName, TEXT("%hS"), name);
-       if (!GetAccountSid(NULL, AccountName, &pSid))
+       n = wnsprintf(AccountName, sizeof(AccountName), TEXT("%hS"), name);
+       if (n < 0 || (size_t)n >= sizeof(AccountName)) {
+               LsaClose(PolicyHandle);
+               return (RTN_ERROR);
+       }
+
+       if (!GetAccountSid(NULL, AccountName, &pSid)) {
+               LsaClose(PolicyHandle);
                return (RTN_NOACCOUNT);
+       }
+
        /*
         * Find out what groups the account belongs to
         */
        istatus = isc_ntsecurity_getaccountgroups(name, Accounts, maxAccounts,
                                                  totalAccounts);
-       if (istatus == ISC_R_NOMEMORY)
+       if (istatus == ISC_R_NOMEMORY) {
+               LsaClose(PolicyHandle);
                return (RTN_NOMEMORY);
-       else if (istatus != ISC_R_SUCCESS)
+       } else if (istatus != ISC_R_SUCCESS) {
+               LsaClose(PolicyHandle);
                return (RTN_ERROR);
+       }
 
        Accounts[*totalAccounts] = name; /* Add the account to the list */
        (*totalAccounts)++;
@@ -138,10 +150,17 @@ GetAccountPrivileges(char *name, wchar_t **PrivList, unsigned int *PrivCount,
         * Loop through each Account to get the list of privileges
         */
        for (i = 0; i < *totalAccounts; i++) {
-               wsprintf(AccountName, TEXT("%hS"), Accounts[i]);
-                /* Obtain the SID of the user/group. */
-               if (!GetAccountSid(NULL, AccountName, &pSid))
+               n = wnsprintf(AccountName, sizeof(AccountName), TEXT("%hS"),
+                             Accounts[i]);
+               if (n < 0 || (size_t)n >= sizeof(AccountName)) {
+                       continue;
+               }
+
+               /* Obtain the SID of the user/group. */
+               if (!GetAccountSid(NULL, AccountName, &pSid)) {
                        continue;       /* Try the next one */
+               }
+
                /* Get the Privileges allocated to this SID */
                if ((Status = GetPrivilegesOnAccount(PolicyHandle, pSid,
                        PrivList, PrivCount)) == STATUS_SUCCESS)
@@ -155,6 +174,7 @@ GetAccountPrivileges(char *name, wchar_t **PrivList, unsigned int *PrivCount,
                        continue;       /* Try the next one */
                }
        }
+
        /*
         * Close the policy handle.
         */
@@ -213,6 +233,7 @@ AddPrivilegeToAcccount(LPTSTR name, LPWSTR PrivilegeName) {
        PSID pSid;
        NTSTATUS Status;
        unsigned long err;
+       int n;
 
        /*
         * Open the policy on the target machine.
@@ -224,9 +245,16 @@ AddPrivilegeToAcccount(LPTSTR name, LPWSTR PrivilegeName) {
        /*
         * Let's see if the account exists. Return if not
         */
-       wsprintf(AccountName, TEXT("%hS"), name);
-       if (!GetAccountSid(NULL, AccountName, &pSid))
+       n = wnsprintf(AccountName, sizeof(AccountName), TEXT("%hS"), name);
+       if (n < 0 || (size_t)n >= sizeof(AccountName)) {
+               LsaClose(PolicyHandle);
+               return (RTN_ERROR);
+       }
+
+       if (!GetAccountSid(NULL, AccountName, &pSid)) {
+               LsaClose(PolicyHandle);
                return (RTN_NOACCOUNT);
+       }
 
        err = LsaNtStatusToWinError(SetPrivilegeOnAccount(PolicyHandle,
                pSid, PrivilegeName, TRUE));

diff --git a/bin/win32/BINDInstall/BINDInstallDlg.cpp b/bin/win32/BINDInstall/BINDInstallDlg.cpp
index 341c427b2e882332ce867f4903bbb5f0dcf0e914..bb38a535aa17a719fb9f8a838f16c6cfa9c7ecd8 100644 (file)
--- a/bin/win32/BINDInstall/BINDInstallDlg.cpp
+++ b/bin/win32/BINDInstall/BINDInstallDlg.cpp
@@ -228,7 +228,6 @@ CBINDInstallDlg::CBINDInstallDlg(CWnd* pParent /*=NULL*/)
        m_startOnInstall = FALSE;
        m_accountName = _T("");
        m_accountPassword = _T("");
-       m_accountName = _T("");
        //}}AFX_DATA_INIT
        // Note that LoadIcon does not require a subsequent
        // DestroyIcon in Win32
@@ -470,6 +469,7 @@ void CBINDInstallDlg::OnUninstall() {
 void CBINDInstallDlg::OnInstall() {
        BOOL success = FALSE;
        int oldlen;
+       int n;
 
        if (CheckBINDService())
                StopBINDService();
@@ -578,12 +578,15 @@ void CBINDInstallDlg::OnInstall() {
        if (runvcredist) {
                char Vcredist_x86[MAX_PATH];
                if (forwin64)
-                       sprintf(Vcredist_x86, "\"%s\\Vcredist_x64.exe\"",
-                               (LPCTSTR) m_currentDir);
+                       n = snprintf(Vcredist_x86, sizeof(Vcredist_x86),
+                                    "\"%s\\Vcredist_x64.exe\"",
+                                    (LPCTSTR) m_currentDir);
                else
-                       sprintf(Vcredist_x86, "\"%s\\Vcredist_x86.exe\"",
-                               (LPCTSTR) m_currentDir);
-               system(Vcredist_x86);
+                       n = snprintf(Vcredist_x86, sizeof(Vcredist_x86),
+                                    "\"%s\\Vcredist_x86.exe\"",
+                                    (LPCTSTR) m_currentDir);
+               if (n >= 0 && (size_t)n < sizeof(Vcredist_x86))
+                       system(Vcredist_x86);
        }
        try {
                CreateDirs();
@@ -1158,8 +1161,13 @@ void CBINDInstallDlg::RegisterMessages() {
        HKEY hKey;
        DWORD dwData;
        char pszMsgDLL[MAX_PATH];
+       int n;
 
-       sprintf(pszMsgDLL, "%s\\%s", (LPCTSTR)m_binDir, "bindevt.dll");
+       n = snprintf(pszMsgDLL, sizeof(pszMsgDLL), "%s\\%s",
+                    (LPCTSTR)m_binDir, "bindevt.dll");
+       if (n < 0 || (size_t)n >= sizeof(pszMsgDLL))
+               throw(Exception(IDS_ERR_CREATE_KEY,
+                               "<m_binDir>\\bindevt.dll too long"));
 
        SetCurrent(IDS_REGISTER_MESSAGES);
        /* Create a new key for named */
@@ -1282,7 +1290,8 @@ void CBINDInstallDlg::SetCurrent(int id, ...) {
        memset(buf, 0, 128);
 
        va_start(va, id);
-       vsprintf(buf, format, va);
+       (void)vsnprintf(buf, sizeof(buf), format, va);
+       buf[sizeof(buf) - 1] = 0;
        va_end(va);
 
        m_current.Format("%s", buf);
@@ -1365,7 +1374,8 @@ int CBINDInstallDlg::MsgBox(int id, ...) {
        memset(buf, 0, BUFSIZ);
 
        va_start(va, id);
-       vsprintf(buf, format, va);
+       (void)vsnprintf(buf, sizeof(buf), format, va);
+       buf[sizeof(buf) - 1] = 0;
        va_end(va);
 
        return (MessageBox(buf));
@@ -1380,7 +1390,8 @@ int CBINDInstallDlg::MsgBox(int id, UINT type, ...) {
        memset(buf, 0, BUFSIZ);
 
        va_start(va, type);
-       vsprintf(buf, format, va);
+       (void)vsnprintf(buf, sizeof(buf), format, va);
+       buf[sizeof(buf) - 1] = 0;
        va_end(va);
 
        return(MessageBox(buf, NULL, type));
@@ -1404,102 +1415,162 @@ CString CBINDInstallDlg::GetErrMessage(DWORD err) {
        return(buf);
 }
 
-void CBINDInstallDlg::ProgramGroup(BOOL create) {
-       TCHAR path[MAX_PATH], commonPath[MAX_PATH], fileloc[MAX_PATH], linkpath[MAX_PATH];
+void CBINDInstallDlg::ProgramGroupCreate(TCHAR *commonPath) {
        HRESULT hres;
        IShellLink *psl = NULL;
-       LPMALLOC pMalloc = NULL;
        ITEMIDLIST *itemList = NULL;
+       TCHAR fileloc[MAX_PATH];
+       TCHAR linkpath[MAX_PATH];
+       TCHAR path[MAX_PATH];
+       int n;
 
-       HRESULT hr = SHGetMalloc(&pMalloc);
-       if (hr != NOERROR) {
-               MessageBox("Could not get a handle to Shell memory object");
+       n = snprintf(path, sizeof(path), "%s\\ISC", commonPath);
+       if (n < 0 || (size_t)n >= sizeof(path))
                return;
-       }
+       CreateDirectory(path, NULL);
 
-       hr = SHGetSpecialFolderLocation(m_hWnd, CSIDL_COMMON_PROGRAMS, &itemList);
-       if (hr != NOERROR) {
-               MessageBox("Could not get a handle to the Common Programs folder");
-               if (itemList) {
-                       pMalloc->Free(itemList);
-               }
+       n = snprintf(path, sizeof(path), "%s\\ISC\\BIND", commonPath);
+       if (n < 0 || (size_t)n >= sizeof(path))
+               return;
+       CreateDirectory(path, NULL);
+
+       hres = CoInitialize(NULL);
+       if (!SUCCEEDED(hres))
                return;
+
+       // Get a pointer to the IShellLink interface.
+       hres = CoCreateInstance(CLSID_ShellLink, NULL, CLSCTX_INPROC_SERVER,
+                               IID_IShellLink, (LPVOID *)&psl);
+       if (!SUCCEEDED(hres)) {
+               goto cleanup;
        }
 
-       hr = SHGetPathFromIDList(itemList, commonPath);
-       pMalloc->Free(itemList);
+       IPersistFile* ppf;
+       n = snprintf(linkpath, sizeof(linkpath), "%s\\BINDCtrl.lnk", path);
+       if (n < 0 || (size_t)n >= sizeof(path)) {
+               goto cleanup;
+       }
 
-       if (create) {
-               sprintf(path, "%s\\ISC", commonPath);
-               CreateDirectory(path, NULL);
+       n = snprintf(fileloc, sizeof(fileloc), "%s\\BINDCtrl.exe",
+                    (LPCTSTR) m_binDir);
+       if (n < 0 || (size_t)n >= sizeof(path)) {
+               goto cleanup;
+       }
 
-               sprintf(path, "%s\\ISC\\BIND", commonPath);
-               CreateDirectory(path, NULL);
+       psl->SetPath(fileloc);
+       psl->SetDescription("BIND Control Panel");
 
-               hres = CoInitialize(NULL);
+       hres = psl->QueryInterface(IID_IPersistFile, (void **)&ppf);
+       if (SUCCEEDED(hres)) {
+               WCHAR wsz[MAX_PATH];
 
-               if (SUCCEEDED(hres)) {
-                       // Get a pointer to the IShellLink interface.
-                       hres = CoCreateInstance(CLSID_ShellLink, NULL, CLSCTX_INPROC_SERVER, IID_IShellLink, (LPVOID *)&psl);
-                       if (SUCCEEDED(hres))
-                       {
-                               IPersistFile* ppf;
-                               sprintf(linkpath, "%s\\BINDCtrl.lnk", path);
-                               sprintf(fileloc, "%s\\BINDCtrl.exe", (LPCTSTR) m_binDir);
+               MultiByteToWideChar(CP_ACP, 0, linkpath, -1, wsz, MAX_PATH);
+               hres = ppf->Save(wsz, TRUE);
+               ppf->Release();
+       }
 
-                               psl->SetPath(fileloc);
-                               psl->SetDescription("BIND Control Panel");
+       if (GetFileAttributes("readme.txt") == -1) {
+               goto cleanup;
+       }
 
-                               hres = psl->QueryInterface(IID_IPersistFile, (void **)&ppf);
-                               if (SUCCEEDED(hres)) {
-                                       WCHAR wsz[MAX_PATH];
+       n = snprintf(fileloc, sizeof(fileloc), "%s\\Readme.txt",
+                    (LPCTSTR) m_targetDir);
+       if (n < 0 || (size_t)n >= sizeof(fileloc)) {
+               goto cleanup;
+       }
 
-                                       MultiByteToWideChar(CP_ACP, 0, linkpath, -1, wsz, MAX_PATH);
-                                       hres = ppf->Save(wsz, TRUE);
-                                       ppf->Release();
-                               }
+       n = snprintf(linkpath, sizeof(linkpath), "%s\\Readme.lnk", path);
+       if (n < 0 || (size_t)n >= sizeof(linkpath)) {
+               goto cleanup;
+       }
 
-                               if (GetFileAttributes("readme.txt") != -1) {
-                                       sprintf(fileloc, "%s\\Readme.txt", (LPCTSTR) m_targetDir);
-                                       sprintf(linkpath, "%s\\Readme.lnk", path);
+       psl->SetPath(fileloc);
+       psl->SetDescription("BIND Readme");
 
-                                       psl->SetPath(fileloc);
-                                       psl->SetDescription("BIND Readme");
+       hres = psl->QueryInterface(IID_IPersistFile, (void **)&ppf);
+       if (SUCCEEDED(hres)) {
+               WCHAR wsz[MAX_PATH];
 
-                                       hres = psl->QueryInterface(IID_IPersistFile, (void **)&ppf);
-                                       if (SUCCEEDED(hres)) {
-                                               WCHAR wsz[MAX_PATH];
+               MultiByteToWideChar(CP_ACP, 0, linkpath, -1, wsz, MAX_PATH);
+               hres = ppf->Save(wsz, TRUE);
+               ppf->Release();
+       }
 
-                                               MultiByteToWideChar(CP_ACP, 0, linkpath, -1, wsz, MAX_PATH);
-                                               hres = ppf->Save(wsz, TRUE);
-                                               ppf->Release();
-                                       }
-                                       psl->Release();
-                               }
+ cleanup:
+       if (psl)
+               psl->Release();
+       CoUninitialize();
+}
+
+void CBINDInstallDlg::ProgramGroupRemove(TCHAR *commonPath) {
+       HANDLE hFind;
+       TCHAR filename[MAX_PATH];
+       TCHAR path[MAX_PATH];
+       WIN32_FIND_DATA fd;
+       int n;
+
+       n = snprintf(path, sizeof(path), "%s\\ISC\\BIND", commonPath);
+       if (n < 0 || (size_t)n >= sizeof(path))
+               goto remove_isc;
+
+       n = snprintf(filename, sizeof(filename), "%s\\*.*", path);
+       if (n < 0 || (size_t)n >= sizeof(path))
+               goto remove_isc_bind;
+
+       hFind = FindFirstFile(filename, &fd);
+       if (hFind != INVALID_HANDLE_VALUE) {
+               do {
+                       if (strcmp(fd.cFileName, ".") == 0 ||
+                           strcmp(fd.cFileName, "..") == 0)
+                               continue;
+                       n = snprintf(filename, sizeof(filename), "%s\\%s",
+                                    path, fd.cFileName);
+                       if (n >= 0 && (size_t)n < sizeof(filename)) {
+                               DeleteFile(filename);
                        }
-                       CoUninitialize();
-               }
+               } while (FindNextFile(hFind, &fd));
+               FindClose(hFind);
        }
-       else {
-               TCHAR filename[MAX_PATH];
-               WIN32_FIND_DATA fd;
-
-               sprintf(path, "%s\\ISC\\BIND", commonPath);
-
-               sprintf(filename, "%s\\*.*", path);
-               HANDLE hFind = FindFirstFile(filename, &fd);
-               if (hFind != INVALID_HANDLE_VALUE) {
-                       do {
-                               if (strcmp(fd.cFileName, ".") && strcmp(fd.cFileName, "..")) {
-                                       sprintf(filename, "%s\\%s", path, fd.cFileName);
-                                       DeleteFile(filename);
-                               }
-                       } while (FindNextFile(hFind, &fd));
-                       FindClose(hFind);
-               }
-               RemoveDirectory(path);
-               sprintf(path, "%s\\ISC", commonPath);
+
+ remove_isc_bind:
+       RemoveDirectory(path);
+
+ remove_isc:
+       n = snprintf(path, sizeof(path), "%s\\ISC", commonPath);
+       if (n >= 0 && (size_t)n < sizeof(path))
                RemoveDirectory(path);
+}
+
+void CBINDInstallDlg::ProgramGroup(BOOL create) {
+       HRESULT hr;
+       ITEMIDLIST *itemList = NULL;
+       LPMALLOC pMalloc = NULL;
+       TCHAR commonPath[MAX_PATH];
+
+       hr = SHGetMalloc(&pMalloc);
+       if (hr != NOERROR) {
+               MessageBox("Could not get a handle to Shell memory object");
+               return;
+       }
+
+       hr = SHGetSpecialFolderLocation(m_hWnd, CSIDL_COMMON_PROGRAMS,
+                                       &itemList);
+       if (hr != NOERROR) {
+               MessageBox("Could not get a handle to the Common Programs "
+                          "folder");
+               if (itemList) {
+                       pMalloc->Free(itemList);
+               }
+               return;
+       }
+
+       hr = SHGetPathFromIDList(itemList, commonPath);
+       pMalloc->Free(itemList);
+
+       if (create) {
+               ProgramGroupCreate(commonPath);
+       } else {
+               ProgramGroupRemove(commonPath);
        }
 }
 

diff --git a/bin/win32/BINDInstall/BINDInstallDlg.h b/bin/win32/BINDInstall/BINDInstallDlg.h
index 61d085bbf1ef88e17786a741b2102709b4f0fb21..88c020e1a2792c7611db52477b9d281780130733 100644 (file)
--- a/bin/win32/BINDInstall/BINDInstallDlg.h
+++ b/bin/win32/BINDInstall/BINDInstallDlg.h
@@ -86,6 +86,8 @@ protected:
        BOOL CheckBINDService();
        void SetCurrent(int id, ...);
        void ProgramGroup(BOOL create = TRUE);
+       void ProgramGroupCreate(TCHAR *commonPath);
+       void ProgramGroupRemove(TCHAR *commonPath);
 
        HICON m_hIcon;
        CString m_defaultDir;

diff --git a/bin/win32/BINDInstall/VersionInfo.cpp b/bin/win32/BINDInstall/VersionInfo.cpp
index c5dc2dc799a610fc1530af50037d74092c2f060e..fab49bf59449c555f985a89effb3a04f87488b0a 100644 (file)
--- a/bin/win32/BINDInstall/VersionInfo.cpp
+++ b/bin/win32/BINDInstall/VersionInfo.cpp
@@ -7,6 +7,8 @@
 #include "VersionInfo.h"
 #include <winver.h>
 
+#include <config.h>
+
 #ifdef _DEBUG
 #undef THIS_FILE
 static char THIS_FILE[]=__FILE__;
@@ -268,20 +270,25 @@ CString CVersionInfo::QueryStringValue(CString value)
 {
        UINT blobLen = 0;
        LPVOID viBlob = NULL;
+       int n;
 
        if(m_versionInfo)
        {
                char queryString[256];
 
-               // This code page value is for American English.  If you change the resources to be other than that
+               // This code page value is for American English.
+               // If you change the resources to be other than that
                // You probably should change this to match it.
                DWORD codePage = 0x040904B0;
 
-               sprintf(queryString, "\\StringFileInfo\\%08X\\%s",
-                       codePage, (LPCTSTR) value);
-
-               if(VerQueryValue(m_versionInfo, queryString, &viBlob, &blobLen))
-                       return((char *)viBlob);
+               n = snprintf(queryString, sizeof(queryString),
+                            "\\StringFileInfo\\%08X\\%s",
+                            codePage, (LPCTSTR) value);
+               if (n >= 0 && (size_t)n < sizeof(queryString)) {
+                       if(VerQueryValue(m_versionInfo, queryString,
+                                        &viBlob, &blobLen))
+                               return((char *)viBlob);
+               }
        }       
        return("Not Available");
 }