CVE-2017-13083: Download of Code Without Integrity Check (CWE-494)
value: Akeo Consulting Rufus prior to version 2.17.1187 does not adequately validate the integrity of updates downloaded over HTTP, allowing an attacker to easily convince a user to execute arbitrary code
CWE description: The product downloads source code or an executable from a remote location and executes the code without sufficiently verifying the origin and integrity of the code.
CVSS v3 base score: 8.1
Published: 2017-10-18T13:29Z
Repository: https://github.com/pbatard/rufus
Language: C
Files changed: res/localization/rufus.loc, src/pki.c, src/rufus.rc, src/stdlg.c
Commit date: 2017-08-31 12:19:11 +0100
Commit message: [pki] fix https://www.kb.cert.org/vuls/id/403768

* This commit effectively fixes https://www.kb.cert.org/vuls/id/403768 (CVE-2017-13083) as
  it is described per its revision 11, which is the latest revision at the time of this commit,
  by disabling Windows prompts, enacted during signature validation, that allow the user to
  bypass the intended signature verification checks.
* It needs to be pointed out that the vulnerability ("allow(ing) the use of a self-signed
  certificate"), which relies on the end-user actively ignoring a Windows prompt that tells
  them that the update failed the signature validation whilst also advising against running it,
  is being fully addressed, even as the update protocol remains HTTP.
* It also need to be pointed out that the extended delay (48 hours) between the time the
  vulnerability was reported and the moment it is fixed in our codebase has to do with
  the fact that the reporter chose to deviate from standard security practices by not
  disclosing the details of the vulnerability with us, be it publicly or privately,
  before creating the cert.org report. The only advance notification we received was a
  generic note about the use of HTTP vs HTTPS, which, as have established, is not
  immediately relevant to addressing the reported vulnerability.
* Closes #1009
* Note: The other vulnerability scenario described towards the end of #1009, which
  doesn't have to do with the "lack of CA checking", will be addressed separately.

Diff: diff --git a/res/localization/rufus.loc b/res/localization/rufus.loc
index 0d14eec..625566f 100644
--- a/res/localization/rufus.loc
+++ b/res/localization/rufus.loc
@@ -485,10 +485,13 @@ t MSG_235 "Bad Blocks: %s %d/%d - %0.2f%% (%d/%d/%d errors)"
 t MSG_236 "Bad Blocks: Testing with random pattern"
 t MSG_237 "Bad Blocks: Testing with pattern 0x%02X"
 # eg. "Partitioning (MBR)..."
 t MSG_238 "Partitioning (%s)..."
 t MSG_239 "Deleting partitions..."
+t MSG_240 "The signature for the downloaded update can not be validated. This could mean that your "
+	"system is improperly configured for signature validation or indicate a malicious download.\n\n"
+	"The download will be deleted. Please check the log for more details."
 t MSG_241 "Downloading: %0.1f%%"
 t MSG_242 "Failed to download file."
 t MSG_243 "Checking for Rufus updates..."
 t MSG_244 "Updates: Unable to connect to the internet"
 t MSG_245 "Updates: Unable to access version data"
diff --git a/src/pki.c b/src/pki.c
index 25de125..00d0141 100644
--- a/src/pki.c
+++ b/src/pki.c
@@ -51,11 +51,11 @@ typedef struct {
 const char* WinPKIErrorString(void)
 {
 	static char error_string[64];
 	DWORD error_code = GetLastError();
 
-	if ((error_code >> 16) != 0x8009)
+	if (((error_code >> 16) != 0x8009) && ((error_code >> 16) != 0x800B))
 		return WindowsErrorString();
 
 	switch (error_code) {
 	case NTE_BAD_UID:
 		return "Bad UID.";
@@ -111,10 +111,16 @@ const char* WinPKIErrorString(void)
 	case CRYPT_E_NO_VERIFY_USAGE_CHECK:
 	case CRYPT_E_VERIFY_USAGE_OFFLINE:
 		return "Cannot complete usage check.";
 	case CRYPT_E_NO_TRUSTED_SIGNER:
 		return "None of the signers of the cryptographic message or certificate trust list is trusted.";
+	case CERT_E_UNTRUSTEDROOT:
+		return "The root certificate is not trusted.";
+	case TRUST_E_NOSIGNATURE:
+		return "Not digitally signed.";
+	case TRUST_E_EXPLICIT_DISTRUST:
+		return "One of the certificates used was marked as untrusted by the user.";
 	default:
 		static_sprintf(error_string, "Unknown PKI error 0x%08lX", error_code);
 		return error_string;
 	}
 }
@@ -266,18 +272,37 @@ LONG ValidateSignature(HWND hDlg, const char* path)
 		uprintf("PKI: Unable to convert '%s' to UTF16", path);
 		return ERROR_SEVERITY_ERROR | FAC(FACILITY_CERT) | ERROR_NOT_ENOUGH_MEMORY;
 	}
 
 	trust_data.cbStruct = sizeof(trust_data);
-	trust_data.dwUIChoice = WTD_UI_ALL;
+	// NB: WTD_UI_ALL can result in ERROR_SUCCESS even if the signature validation fails,
+	// because it still prompts the user to run untrusted software, even after explicitly
+	// notifying them that the signature invalid (and of course Microsoft had to make
+	// that UI prompt a bit too similar to the other benign prompt you get when running
+	// trusted software, which, as per cert.org's assessment, may confuse non-security
+	// conscious-users who decide to gloss over these kind of notifications).
+	trust_data.dwUIChoice = WTD_UI_NONE;
 	// We just downloaded from the Internet, so we should be able to check revocation
 	trust_data.fdwRevocationChecks = WTD_REVOKE_WHOLECHAIN;
 	// 0x400 = WTD_MOTW  for Windows 8.1 or later
 	trust_data.dwProvFlags = WTD_REVOCATION_CHECK_CHAIN | 0x400;
 	trust_data.dwUnionChoice = WTD_CHOICE_FILE;
 	trust_data.pFile = &trust_file;
 
 	r = WinVerifyTrust(NULL, &guid_generic_verify, &trust_data);
 	safe_free(trust_file.pcwszFilePath);
+	switch (r) {
+	case ERROR_SUCCESS:
+		break;
+	case TRUST_E_NOSIGNATURE:
+		// Should already have been reported, but since we have a custom message for it...
+		uprintf("PKI: File does not appear to be signed: %s", WinPKIErrorString());
+		MessageBoxExU(hDlg, lmprintf(MSG_284), lmprintf(MSG_283), MB_OK | MB_ICONERROR | MB_IS_RTL, selected_langid);
+		break;
+	default:
+		uprintf("PKI: Failed to validate signature: %s", WinPKIErrorString());
+		MessageBoxExU(hDlg, lmprintf(MSG_240), lmprintf(MSG_283), MB_OK | MB_ICONERROR | MB_IS_RTL, selected_langid);
+		break;
+	}
 
 	return r;
 }
diff --git a/src/rufus.rc b/src/rufus.rc
index 3f553ee..0d0c2a7 100644
--- a/src/rufus.rc
+++ b/src/rufus.rc
@@ -31,11 +31,11 @@ LANGUAGE LANG_NEUTRAL, SUBLANG_NEUTRAL
 //
 
 IDD_DIALOG DIALOGEX 12, 12, 242, 376
 STYLE DS_SETFONT | DS_MODALFRAME | DS_CENTER | WS_MINIMIZEBOX | WS_POPUP | WS_CAPTION | WS_SYSMENU
 EXSTYLE WS_EX_ACCEPTFILES
-CAPTION "Rufus 2.17.1186"
+CAPTION "Rufus 2.17.1187"
 FONT 8, "Segoe UI Symbol", 400, 0, 0x0
 BEGIN
     LTEXT           "Device",IDS_DEVICE_TXT,9,6,200,8
     COMBOBOX        IDC_DEVICE,8,17,226,33,CBS_DROPDOWNLIST | WS_VSCROLL | WS_TABSTOP
     LTEXT           "Partition scheme and target system type",IDS_PARTITION_TYPE_TXT,9,35,222,8
@@ -364,12 +364,12 @@ END
 //
 // Version
 //
 
 VS_VERSION_INFO VERSIONINFO
- FILEVERSION 2,17,1186,0
- PRODUCTVERSION 2,17,1186,0
+ FILEVERSION 2,17,1187,0
+ PRODUCTVERSION 2,17,1187,0
  FILEFLAGSMASK 0x3fL
 #ifdef _DEBUG
  FILEFLAGS 0x1L
 #else
  FILEFLAGS 0x0L
@@ -382,17 +382,17 @@ BEGIN
     BEGIN
         BLOCK "000004b0"
         BEGIN
             VALUE "CompanyName", "Akeo Consulting (http://akeo.ie)"
             VALUE "FileDescription", "Rufus"
-            VALUE "FileVersion", "2.17.1186"
+            VALUE "FileVersion", "2.17.1187"
             VALUE "InternalName", "Rufus"
             VALUE "LegalCopyright", "© 2011-2017 Pete Batard (GPL v3)"
             VALUE "LegalTrademarks", "http://www.gnu.org/copyleft/gpl.html"
             VALUE "OriginalFilename", "rufus.exe"
             VALUE "ProductName", "Rufus"
-            VALUE "ProductVersion", "2.17.1186"
+            VALUE "ProductVersion", "2.17.1187"
         END
     END
     BLOCK "VarFileInfo"
     BEGIN
         VALUE "Translation", 0x0, 1200
diff --git a/src/stdlg.c b/src/stdlg.c
index 29eccae..bdec209 100644
--- a/src/stdlg.c
+++ b/src/stdlg.c
@@ -1672,12 +1672,16 @@ INT_PTR CALLBACK NewVersionCallback(HWND hDlg, UINT message, WPARAM wParam, LPAR
 				download_status = 0;
 				break;
 			case 2:		// Launch newer version and close this one
 				Sleep(1000);	// Add a delay on account of antivirus scanners
 
-				if (ValidateSignature(hDlg, filepath) != NO_ERROR)
+				if (ValidateSignature(hDlg, filepath) != NO_ERROR) {
+					// Unconditionally delete the download and disable the "Launch" control
+					_unlinkU(filepath);
+					EnableWindow(GetDlgItem(hDlg, IDC_DOWNLOAD), FALSE);
 					break;
+				}
 
 				memset(&si, 0, sizeof(si));
 				memset(&pi, 0, sizeof(pi));
 				si.cb = sizeof(si);
 				if (!CreateProcessU(filepath, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi)) {

Vulnerable code: 	if ((error_code >> 16) != 0x8009)
	trust_data.dwUIChoice = WTD_UI_ALL;
CAPTION "Rufus 2.17.1186"
 FILEVERSION 2,17,1186,0
 PRODUCTVERSION 2,17,1186,0
            VALUE "FileVersion", "2.17.1186"
            VALUE "ProductVersion", "2.17.1186"
				if (ValidateSignature(hDlg, filepath) != NO_ERROR)
Fixed code: t MSG_240 "The signature for the downloaded update can not be validated. This could mean that your "
	"system is improperly configured for signature validation or indicate a malicious download.\n\n"
	"The download will be deleted. Please check the log for more details."
	if (((error_code >> 16) != 0x8009) && ((error_code >> 16) != 0x800B))
	case CERT_E_UNTRUSTEDROOT:
		return "The root certificate is not trusted.";
	case TRUST_E_NOSIGNATURE:
		return "Not digitally signed.";
	case TRUST_E_EXPLICIT_DISTRUST:
		return "One of the certificates used was marked as untrusted by the user.";
	// NB: WTD_UI_ALL can result in ERROR_SUCCESS even if the signature validation fails,
	// because it still prompts the user to run untrusted software, even after explicitly
	// notifying them that the signature invalid (and of course Microsoft had to make
	// that UI prompt a bit too similar to the other benign prompt you get when running
	// trusted software, which, as per cert.org's assessment, may confuse non-security
	// conscious-users who decide to gloss over these kind of notifications).
	trust_data.dwUIChoice = WTD_UI_NONE;
	switch (r) {
	case ERROR_SUCCESS:
		break;
	case TRUST_E_NOSIGNATURE:
		// Should already have been reported, but since we have a custom message for it...
		uprintf("PKI: File does not appear to be signed: %s", WinPKIErrorString());
		MessageBoxExU(hDlg, lmprintf(MSG_284), lmprintf(MSG_283), MB_OK | MB_ICONERROR | MB_IS_RTL, selected_langid);
		break;
	default:
		uprintf("PKI: Failed to validate signature: %s", WinPKIErrorString());
		MessageBoxExU(hDlg, lmprintf(MSG_240), lmprintf(MSG_283), MB_OK | MB_ICONERROR | MB_IS_RTL, selected_langid);
		break;
	}
CAPTION "Rufus 2.17.1187"
 FILEVERSION 2,17,1187,0
 PRODUCTVERSION 2,17,1187,0
            VALUE "FileVersion", "2.17.1187"
            VALUE "ProductVersion", "2.17.1187"
				if (ValidateSignature(hDlg, filepath) != NO_ERROR) {
					// Unconditionally delete the download and disable the "Launch" control
					_unlinkU(filepath);
					EnableWindow(GetDlgItem(hDlg, IDC_DOWNLOAD), FALSE);
				}