From c28e44f3f8bc46c6bf1c15cc06af0c42fcd7e924 Mon Sep 17 00:00:00 2001
From: Evan Hunt <each@isc.org>
Date: Tue, 30 May 2017 13:38:22 -0700
Subject: [PATCH] [v9_11] quote service registry paths

4532.	[security]	The BIND installer on Windows used an unquoted
                        service path, which can enable privilege escalation.
			(CVE-2017-3141) [RT #45229]

(cherry picked from commit 967a3b9419a3c12b8c0870c86d1ee3840bcbbad7)
---
 CHANGES                                  |  4 ++++
 README                                   |  8 +++----
 README.md                                |  8 +++----
 bin/win32/BINDInstall/BINDInstallDlg.cpp | 29 +++++++++++++++++++++---
 doc/arm/notes.xml                        |  7 ++++++
 5 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/CHANGES b/CHANGES
index 7b0d6368b8..9c89bb89a5 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,7 @@
+4532.	[security]	The BIND installer on Windows used an unquoted
+                        service path, which can enable privilege escalation.
+			(CVE-2017-3141) [RT #45229]
+
 4531.	[security]	Some RPZ configurations could go into an infinite
 			query loop when encountering responses with TTL=0.
 			(CVE-2017-3140) [RT #45181]
diff --git a/README b/README
index 88438e0cd4..c0228f4014 100644
--- a/README
+++ b/README
@@ -224,10 +224,10 @@ and CVE-2017-3138.
 
 BIND 9.11.2
 
-BIND 9.11.1 is a maintenance release, and addresses the security flaw
-disclosed in CVE-2017-3140. It also addresses several bugs related to the
-use of an LMDB database to store data related to zones added via rndc
-addzone or catalog zones.
+BIND 9.11.1 is a maintenance release, and addresses the security flaws
+disclosed in CVE-2017-3140 and CVE-2017-3141. It also addresses several
+bugs related to the use of an LMDB database to store data related to zones
+added via rndc addzone or catalog zones.
 
 Building BIND
 
diff --git a/README.md b/README.md
index a85f9a410b..9844570cce 100644
--- a/README.md
+++ b/README.md
@@ -236,10 +236,10 @@ CVE-2017-3136, CVE-2017-3137 and CVE-2017-3138.
 
 #### BIND 9.11.2
 
-BIND 9.11.1 is a maintenance release, and addresses the security flaw
-disclosed in CVE-2017-3140. It also addresses several bugs related to
-the use of an LMDB database to store data related to zones added via
-`rndc addzone` or catalog zones.
+BIND 9.11.1 is a maintenance release, and addresses the security flaws
+disclosed in CVE-2017-3140 and CVE-2017-3141. It also addresses several
+bugs related to the use of an LMDB database to store data related to
+zones added via `rndc addzone` or catalog zones.
 
 ### <a name="build"/> Building BIND
 
diff --git a/bin/win32/BINDInstall/BINDInstallDlg.cpp b/bin/win32/BINDInstall/BINDInstallDlg.cpp
index 041f651a1b..3569f8d352 100644
--- a/bin/win32/BINDInstall/BINDInstallDlg.cpp
+++ b/bin/win32/BINDInstall/BINDInstallDlg.cpp
@@ -50,6 +50,7 @@
 #include "DirBrowse.h"
 #include <winsvc.h>
 #include <shlobj.h>
+#include <shlwapi.h>
 #include <named/ntservice.h>
 #include <isc/bind_registry.h>
 #include <isc/ntgroups.h>
@@ -615,8 +616,16 @@ void CBINDInstallDlg::OnInstall() {
 					(LPBYTE)(LPCTSTR)buf, buf.GetLength());
 
 			buf.Format("%s\\BINDInstall.exe", m_binDir);
+
+			CStringA installLocA(buf);
+			const char *str = (const char *) installLocA;
+			char pathBuffer[2 * MAX_PATH];
+			strncpy(pathBuffer, str, sizeof(pathBuffer) - 1);
+			pathBuffer[sizeof(pathBuffer) - 1] = 0;
+			PathQuoteSpaces(pathBuffer);
+
 			RegSetValueEx(hKey, "UninstallString", 0, REG_SZ,
-					(LPBYTE)(LPCTSTR)buf, buf.GetLength());
+				      (LPBYTE)(LPCTSTR)pathBuffer, strlen(pathBuffer));
 			RegCloseKey(hKey);
 		}
 
@@ -1011,10 +1020,17 @@ CBINDInstallDlg::RegisterService() {
 	CString namedLoc;
 	namedLoc.Format("%s\\bin\\named.exe", m_targetDir);
 
+	CStringA namedLocA(namedLoc);
+	const char *str = (const char *) namedLocA;
+	char pathBuffer[2 * MAX_PATH];
+	strncpy(pathBuffer, str, sizeof(pathBuffer) - 1);
+	pathBuffer[sizeof(pathBuffer) - 1] = 0;
+	PathQuoteSpaces(pathBuffer);
+
 	SetCurrent(IDS_CREATE_SERVICE);
 	hService = CreateService(hSCManager, BIND_SERVICE_NAME,
 		BIND_DISPLAY_NAME, SERVICE_ALL_ACCESS, dwServiceType, dwStart,
-		SERVICE_ERROR_NORMAL, namedLoc, NULL, NULL, NULL, StartName,
+		SERVICE_ERROR_NORMAL, pathBuffer, NULL, NULL, NULL, StartName,
 		m_accountPassword);
 
 	if (!hService && GetLastError() != ERROR_SERVICE_EXISTS)
@@ -1053,6 +1069,13 @@ CBINDInstallDlg::UpdateService(CString StartName) {
 	CString namedLoc;
 	namedLoc.Format("%s\\bin\\named.exe", m_targetDir);
 
+	CStringA namedLocA(namedLoc);
+	const char *str = (const char *) namedLocA;
+	char pathBuffer[2 * MAX_PATH];
+	strncpy(pathBuffer, str, sizeof(pathBuffer) - 1);
+	pathBuffer[sizeof(pathBuffer) - 1] = 0;
+	PathQuoteSpaces(pathBuffer);
+
 	SetCurrent(IDS_OPEN_SERVICE);
 	hService = OpenService(hSCManager, BIND_SERVICE_NAME,
 			       SERVICE_CHANGE_CONFIG);
@@ -1064,7 +1087,7 @@ CBINDInstallDlg::UpdateService(CString StartName) {
 		return;
 	} else {
 		if (ChangeServiceConfig(hService, dwServiceType, dwStart,
-			SERVICE_ERROR_NORMAL, namedLoc, NULL, NULL, NULL,
+			SERVICE_ERROR_NORMAL, pathBuffer, NULL, NULL, NULL,
 			StartName, m_accountPassword, BIND_DISPLAY_NAME)
 			!= TRUE) {
 			DWORD err = GetLastError();
diff --git a/doc/arm/notes.xml b/doc/arm/notes.xml
index 8202ca4062..1b623584a6 100644
--- a/doc/arm/notes.xml
+++ b/doc/arm/notes.xml
@@ -93,6 +93,13 @@
 
   <section xml:id="relnotes_security"><info><title>Security Fixes</title></info>
     <itemizedlist>
+      <listitem>
+	<para>
+	  The BIND installer on Windows used an unquoted service path,
+	  which can enable privilege escalation. This flaw is disclosed
+	  in CVE-2017-3141. [RT #45229]
+	</para>
+      </listitem>
       <listitem>
 	<para>
 	  With certain RPZ configurations, a response with TTL 0