Veracrypt Inserts Invalid Text in the Boot Loader Configuration Editor Screen

[gashtal]

Veracrypt's Boot Loader Configuration Editor inserts extra text to the config file when it is opened in the editor and the XML format validator fails when the file is modified and then saved, unless the random text added by Veracrypt is removed manually.

Expected behavior

Veracrypt's Boot Loader Configuration Editor screen should only show text that is actually in the config file.

Observed behavior

Veracrypt's Boot Loader Configuration Editor inserts extra text to the config file when it is opened in the editor.

Steps to reproduce

1. Encrypt system partition.
2. Open the Boot Loader Configuration Editor from System > Settings > Edit Bootloader Configuration.
3. Observe that there is extra random text added at the bottom as shown below:

4. Modify any settings and press "OK".
5. You will get the XML format validation error.
6. Remove the random text at the bottom and press "OK".
7. File will be saved successfully.

Your Environment

Please tell us more about your environment

VeraCrypt version: 1.25.9

Operating system and version: Windows 10 21H1

System type: 64-bit

[aetonsi]

the internal editor really messes up DcsProp; editing that file with that editor it's really risky.... doing this is better (as admin): mountvol /s v:, v:, cd EFI\VeraCrypt, notepad DcsProp

[kriegste]

I can confirm this. It happens if you delete something in the config file. The new (shorter) content overwrites the old (longer) file, but the leftover text at the end is still present. As if the string buffer is copied over the old one without a \0 termination char and then saved.

Edit: Oh, and also the "OK" and "Cancel" button captions do not get translated. They are also swapped ("Cancel" on the left, "OK" on the right) compared to the window before. That is a bit misleading.

[kriegste]

My guess is that the problem is here somewhere: https://github.com/veracrypt/VeraCrypt/blob/master/src/Common/BootEncryption.cpp#L2589

Data length is obtained using strlen (in Mount.c), which returns the pure string length omitting the terminator \0.

[idrassi]

Thank you @aetonsi and @kriegste for reviving this issue.

I finally took the time to look into it and it was caused by issues in UI code in TextEditDlgProc function in Dlgcode.c. I have pushed a commit that fixes the issues: https://github.com/veracrypt/VeraCrypt/commit/68e2e017450343b5a36d1179616129a53ed8d9c2

Here is what this commit does:

• We always using Unicode functions to interact with UI. We convert UTF8 string to UTF16 and vis-versa.
• Overwrite input string instead of using resize that caused old text to remain.
• Fix case of readOnly by using correct message to set RichEdit as readOnly.
• Change position of OK/cancel button to match other dialogs.
• Activate translation for this dialog.

Based on my test, things should be good now.

[chrdev]

I'm using 1.26.20. I think there is another related issue.

The edit dialog is alright, the culprit is the file writing. If the user edited DcsProp to a smaller size, the original file is only partially overwritten, leaving trash at the file tail, thus a malformed xml.

The fix should be easy, just truncate the file after writing by calling SetEndOfFile().

My proposed fix:

diff --git a/src/Common/BootEncryption.cpp b/src/Common/BootEncryption.cpp
index 833a67fd..95442c1a 100644
--- a/src/Common/BootEncryption.cpp
+++ b/src/Common/BootEncryption.cpp
@@ -877,6 +877,16 @@ namespace VeraCrypt
 		}
 	}
 
+	void File::SetEnd()
+	{
+		if (!FileOpen)
+		{
+			SetLastError (LastError);
+			throw SystemException (SRC_POS);
+		}
+		SetEndOfFile (Handle);
+	}
+
 	void File::GetFileSize (unsigned __int64& size)
 	{
 		if (!FileOpen)
@@ -2599,6 +2609,7 @@ namespace VeraCrypt
 
 		File f(pathESP + szFilePath, false, true);
 		f.Write(fileContent.data(), dwSize);
+		f.SetEnd();
 		f.Close();
 
 	}
diff --git a/src/Common/BootEncryption.h b/src/Common/BootEncryption.h
index 432d10a9..f41e9c9c 100644
--- a/src/Common/BootEncryption.h
+++ b/src/Common/BootEncryption.h
@@ -40,6 +40,7 @@ namespace VeraCrypt
 		DWORD Read (uint8 *buffer, DWORD size);
 		void Write (uint8 *buffer, DWORD size);
 		void SeekAt (int64 position);
+		void SetEnd ();
 		void GetFileSize (unsigned __int64& size);
 		void GetFileSize (DWORD& dwSize);
       bool IoCtl(DWORD code, void* inBuf, DWORD inBufSize, void* outBuf, DWORD outBufSize);

[aetonsi]

The edit dialog is alright, the culprit is the file writing. If the user edited DcsProp to a smaller size, the original file is only partially overwritten, leaving trash at the file tail, thus a malformed xml.

I agree on the culprit being the file writing, also because it breaks html entities, converting them to the actual character instead of leaving it literal, like it should be..

for example if i use a > to represent a >:

<config key="PasswordMsg">&gt; pwd: </config>

it gets converted to an actual > and breaks the entire xml structure:

<config key="PasswordMsg">> pwd: </config>