New features added to VeraCryptFormat.dll#1659
New features added to VeraCryptFormat.dll#1659YonyUk wants to merge 3 commits intoveracrypt:masterfrom
Conversation
|
Thank you for the contribution. Main blockers:
options->autoDetectEncryptionAlgorithm ? 0 : static_cast<int>(options->encryptionAlgorithm)That is an encryption algorithm enum value, not a PRF/hash id. The mount API should expose a PRF/hash option or use 0 for autodetection. Encryption algorithm selection is not the right parameter here.
MountVolume returns:
But VeraCryptMount returns raw 0 when MountVolume fails, and 0 is VCF_SUCCESS. This means a wrong password or normal mount failure can be reported to SDK callers as success. This is a critical bug and needs explicit mapping to nonzero SDK error codes.
The header uses declarations such as: typedef enum DriveLetter { This does not create a C typedef named DriveLetter: it only defines enum DriveLetter. Later declarations like DriveLetter letter; therefore do not work for C consumers. Since this is a C-style DLL API, the header must be valid C as well as C++.
VeraCryptFormatOptions is changed in place: string fields are replaced with enums and a new sizeMeasureUnity field is inserted. Existing clients compiled against the current SDK header would pass the old struct layout to the same exported VeraCryptFormat function, and the DLL would misinterpret the fields. Please either preserve the existing struct/API or introduce a versioned API, for example VeraCryptFormatOptionsV2 plus VeraCryptFormatEx, preferably with a structSize field for forward compatibility.
The PR indexes arrays with values coming from public enum fields: VolumeEncryptionAlgorithmOptions[static_cast(options->encryptionAlgorithm)] For a C ABI, callers can pass any integer value. These must be bounds-checked before indexing.
Current master supports SHA-512, SHA-256, BLAKE2s-256, Whirlpool, Streebog, and BLAKE2b-512/Argon2 in the current hash table. The PR exposes RIPEMD-160, which is no longer present in the current table and omits Streebog and BLAKE2b-512.
VeraCryptMount dereferences options before checking it, does not validate options->path and passes the caller-provided path buffer directly to MountVolume. Suggested way forward:
I recommend reworking the API and implementation around these points before continuing with merge review. |
idrassi
left a comment
idrassi
left a comment
There was a problem hiding this comment.
Requesting changes because the current implementation has several merge-blocking API and correctness issues: the public C header is not valid C, the existing VeraCryptFormatOptions ABI is broken in place, mount passes an encryption enum as the PRF/hash parameter, mount failures can be returned as VCF_SUCCESS, public enum values are used as unchecked array indexes, and VeraCryptMount lacks basic input/path validation.
The feature direction is useful, but this should be reworked around a versioned SDK API, validated enum mappings, correct mount PRF handling, explicit SDK error mapping, and safe copying of mutable inputs before calling internal VeraCrypt functions.
| typedef enum DriveLetter { | ||
| DRIVE_LETTER_A = 0, | ||
| DRIVE_LETTER_B, | ||
| DRIVE_LETTER_C, | ||
| DRIVE_LETTER_D, | ||
| DRIVE_LETTER_E, | ||
| DRIVE_LETTER_F, | ||
| DRIVE_LETTER_G, | ||
| DRIVE_LETTER_H, | ||
| DRIVE_LETTER_I, | ||
| DRIVE_LETTER_J, | ||
| DRIVE_LETTER_K, | ||
| DRIVE_LETTER_L, | ||
| DRIVE_LETTER_M, | ||
| DRIVE_LETTER_N, | ||
| DRIVE_LETTER_O, | ||
| DRIVE_LETTER_P, | ||
| DRIVE_LETTER_Q, | ||
| DRIVE_LETTER_R, | ||
| DRIVE_LETTER_S, | ||
| DRIVE_LETTER_T, | ||
| DRIVE_LETTER_U, | ||
| DRIVE_LETTER_V, | ||
| DRIVE_LETTER_W, | ||
| DRIVE_LETTER_X, | ||
| DRIVE_LETTER_Y, | ||
| DRIVE_LETTER_Z | ||
| }; |
There was a problem hiding this comment.
This breaks the public C header. In C, typedef enum DriveLetter { ... }; does not create a typedef named DriveLetter; it only declares enum DriveLetter. The later fields using DriveLetter, VolumeEncryptionAlgorithm, etc. will not compile for C consumers of this C-style DLL API.
Please use the C-compatible form for all these enums, e.g.:
typedef enum DriveLetter {
...
} DriveLetter;| int pim; | ||
|
|
||
| /** size measure unity */ | ||
| SizeMeasureUnity sizeMeasureUnity; |
There was a problem hiding this comment.
This changes the existing VeraCryptFormatOptions layout in place while keeping the same exported VeraCryptFormat API. Existing SDK clients compiled against the current header will pass the old struct layout, and the new DLL will interpret the following fields incorrectly.
This needs to preserve the existing ABI, or introduce a versioned API/struct, e.g. VeraCryptFormatOptionsV2 plus VeraCryptFormatEx, preferably with a structSize field for forward compatibility.
| typedef enum HashAlgorithm{ | ||
| VERACRYPT_HASH_SHA_512 = 0, | ||
| VERACRYPT_HASH_SHA_256, | ||
| VERACRYPT_HASH_RIPEMD_160, |
There was a problem hiding this comment.
This hash enum does not match current VeraCrypt master. RIPEMD-160 is no longer present in the current hash table, while Streebog and BLAKE2b-512/Argon2 are supported. This would expose an invalid option and omit valid ones.
Please derive this list from the current PRF/hash support or update the enum and mapping to match Crypto.c.
| } | ||
|
|
||
| nVolumeEA = MapEncryptionAlgorithm(options->encryptionAlgorithm); | ||
| nVolumeEA = MapEncryptionAlgorithm(VolumeEncryptionAlgorithmOptions[static_cast<int>(options->encryptionAlgorithm)]); |
There was a problem hiding this comment.
This indexes a fixed array using a public enum value without bounds checking. Since this is a C ABI, callers can pass any integer value, not only valid enum constants. An invalid value here can read out of bounds before MapEncryptionAlgorithm has a chance to reject it.
The same issue exists for HashAlgorithmOptions and FileSystemFormatOptions below. Please validate each enum value before indexing, and return the appropriate VCF_ERROR_INVALID_* code.
| bDevice = options->isDevice; | ||
| nVolumeSize = options->size; | ||
| if (!options->isDevice) { | ||
| nVolumeSize = options->size * (options->sizeMeasureUnity == SizeMeasureUnity::Kilobytes ? |
There was a problem hiding this comment.
This multiplication can overflow uint64_t before the later size checks run. For example, a large options->size value with Gigabytes can wrap to a small value and pass subsequent validation incorrectly.
Please validate sizeMeasureUnity, check options->size <= UINT64_MAX / multiplier, and update the public comment because size is no longer always expressed in bytes.
| KeyFileRemoveAll(&FirstKeyFile); | ||
| }); | ||
|
|
||
| if (!IsDriveAvailable(static_cast<int>(options->letter))) { |
There was a problem hiding this comment.
options is dereferenced here before any null check, and options->path is not validated at all. A null options or null/empty path will crash instead of returning VCF_ERROR_INVALID_PARAMETER.
Please validate options, options->path, and required password/keyfile inputs before using any fields.
| mountOptions.SkipCachedPasswords = options->SkipCachedPasswords; | ||
| mountOptions.UseBackupHeader = options->UseBackupHeader; | ||
|
|
||
| int result = MountVolume(NULL, static_cast<int>(options->letter), options->path, &volumePassword, |
There was a problem hiding this comment.
MountVolume can modify the volumePath buffer it receives. Passing options->path directly makes the SDK mutate caller-owned input, and the public struct does not document that requirement.
Please copy options->path into an SDK-owned writable buffer, sized for TC_MAX_PATH, validate/copy safely, and pass that internal buffer to MountVolume.
| mountOptions.UseBackupHeader = options->UseBackupHeader; | ||
|
|
||
| int result = MountVolume(NULL, static_cast<int>(options->letter), options->path, &volumePassword, | ||
| options->autoDetectEncryptionAlgorithm ? 0 : static_cast<int>(options->encryptionAlgorithm), |
There was a problem hiding this comment.
This passes an encryption algorithm enum where MountVolume expects the pkcs5 PRF/hash id. The fifth parameter of MountVolume is int pkcs5, not an encryption algorithm selector.
The mount API should expose a PRF/hash option, or pass 0 for autodetection. Encryption algorithm selection is not the right parameter for this call.
| if (options->sharedAccess) | ||
| return result == 2 ? VCF_SUCCESS : result; | ||
| return result == 1 ? VCF_SUCCESS : result; | ||
| } |
There was a problem hiding this comment.
This maps mount failures to VCF_SUCCESS. MountVolume returns 0 for mount failure, including wrong password, and VCF_SUCCESS is also 0. As written, a wrong password can be reported to SDK callers as success.
Please map MountVolume results explicitly: 1 and 2 should map to VCF_SUCCESS, while 0 and -1 must map to nonzero SDK error codes.
|
Thanks fourth your revision. I'll be working on it.
El lun., 13 de abril de 2026 2:39 a. m., Mounir IDRASSI <
***@***.***> escribió:
… ***@***.**** requested changes on this pull request.
Requesting changes because the current implementation has several
merge-blocking API and correctness issues: the public C header is not valid
C, the existing VeraCryptFormatOptions ABI is broken in place, mount
passes an encryption enum as the PRF/hash parameter, mount failures can be
returned as VCF_SUCCESS, public enum values are used as unchecked array
indexes, and VeraCryptMount lacks basic input/path validation.
The feature direction is useful, but this should be reworked around a
versioned SDK API, validated enum mappings, correct mount PRF handling,
explicit SDK error mapping, and safe copying of mutable inputs before
calling internal VeraCrypt functions.
------------------------------
In src/FormatDLL/VeraCryptFormatSDK.h
<#1659 (comment)>:
> +typedef enum DriveLetter {
+ DRIVE_LETTER_A = 0,
+ DRIVE_LETTER_B,
+ DRIVE_LETTER_C,
+ DRIVE_LETTER_D,
+ DRIVE_LETTER_E,
+ DRIVE_LETTER_F,
+ DRIVE_LETTER_G,
+ DRIVE_LETTER_H,
+ DRIVE_LETTER_I,
+ DRIVE_LETTER_J,
+ DRIVE_LETTER_K,
+ DRIVE_LETTER_L,
+ DRIVE_LETTER_M,
+ DRIVE_LETTER_N,
+ DRIVE_LETTER_O,
+ DRIVE_LETTER_P,
+ DRIVE_LETTER_Q,
+ DRIVE_LETTER_R,
+ DRIVE_LETTER_S,
+ DRIVE_LETTER_T,
+ DRIVE_LETTER_U,
+ DRIVE_LETTER_V,
+ DRIVE_LETTER_W,
+ DRIVE_LETTER_X,
+ DRIVE_LETTER_Y,
+ DRIVE_LETTER_Z
+};
This breaks the public C header. In C, typedef enum DriveLetter { ... };
does not create a typedef named DriveLetter; it only declares enum
DriveLetter. The later fields using DriveLetter, VolumeEncryptionAlgorithm,
etc. will not compile for C consumers of this C-style DLL API.
Please use the C-compatible form for all these enums, e.g.:
typedef enum DriveLetter {
...
} DriveLetter;
------------------------------
In src/FormatDLL/VeraCryptFormatSDK.h
<#1659 (comment)>:
> @@ -60,17 +137,20 @@ typedef struct _VeraCryptFormatOptions
/** The Personal Iterations Multiplier (PIM). Use 0 for default PIM. */
int pim;
+ /** size measure unity */
+ SizeMeasureUnity sizeMeasureUnity;
This changes the existing VeraCryptFormatOptions layout in place while
keeping the same exported VeraCryptFormat API. Existing SDK clients
compiled against the current header will pass the old struct layout, and
the new DLL will interpret the following fields incorrectly.
This needs to preserve the existing ABI, or introduce a versioned
API/struct, e.g. VeraCryptFormatOptionsV2 plus VeraCryptFormatEx,
preferably with a structSize field for forward compatibility.
------------------------------
In src/FormatDLL/VeraCryptFormatSDK.h
<#1659 (comment)>:
> + VERACRYPT_VOLUME_ENCRYPTION_AES_Serpent,
+ VERACRYPT_VOLUME_ENCRYPTION_Serpent_Twofish_AES,
+ VERACRYPT_VOLUME_ENCRYPTION_Serpent_Twofish,
+ VERACRYPT_VOLUME_ENCRYPTION_Kuznyechik_Camellia,
+ VERACRYPT_VOLUME_ENCRYPTION_Twofish_Kuznyechik,
+ VERACRYPT_VOLUME_ENCRYPTION_Serpent_Camellia,
+ VERACRYPT_VOLUME_ENCRYPTION_AES_Kuznyechik,
+ VERACRYPT_VOLUME_ENCRYPTION_Camellia_Serpent_Kuznyechik
+};
+/**
+* @brief enum with all the hash algorithms aviables for volumen creation
+*/
+typedef enum HashAlgorithm{
+ VERACRYPT_HASH_SHA_512 = 0,
+ VERACRYPT_HASH_SHA_256,
+ VERACRYPT_HASH_RIPEMD_160,
This hash enum does not match current VeraCrypt master. RIPEMD-160 is no
longer present in the current hash table, while Streebog and BLAKE2b-512/Argon2
are supported. This would expose an invalid option and omit valid ones.
Please derive this list from the current PRF/hash support or update the
enum and mapping to match Crypto.c.
------------------------------
In src/FormatDLL/VeraCryptFormatSDK.cpp
<#1659 (comment)>:
>
- nVolumeEA = MapEncryptionAlgorithm(options->encryptionAlgorithm);
+ nVolumeEA = MapEncryptionAlgorithm(VolumeEncryptionAlgorithmOptions[static_cast<int>(options->encryptionAlgorithm)]);
This indexes a fixed array using a public enum value without bounds
checking. Since this is a C ABI, callers can pass any integer value, not
only valid enum constants. An invalid value here can read out of bounds
before MapEncryptionAlgorithm has a chance to reject it.
The same issue exists for HashAlgorithmOptions and FileSystemFormatOptions
below. Please validate each enum value before indexing, and return the
appropriate VCF_ERROR_INVALID_* code.
------------------------------
In src/FormatDLL/VeraCryptFormatSDK.cpp
<#1659 (comment)>:
> @@ -184,15 +219,18 @@ static int CreateVolumeInternal(const VeraCryptFormatOptions* options)
// --- Setup VeraCrypt's global state from our options struct ---
bDevice = options->isDevice;
- nVolumeSize = options->size;
+ if (!options->isDevice) {
+ nVolumeSize = options->size * (options->sizeMeasureUnity == SizeMeasureUnity::Kilobytes ?
This multiplication can overflow uint64_t before the later size checks
run. For example, a large options->size value with Gigabytes can wrap to
a small value and pass subsequent validation incorrectly.
Please validate sizeMeasureUnity, check options->size <= UINT64_MAX /
multiplier, and update the public comment because size is no longer
always expressed in bytes.
------------------------------
In src/FormatDLL/VeraCryptFormatSDK.cpp
<#1659 (comment)>:
> + if (!g_isInitialized)
+ {
+ return VCF_ERROR_NOT_INITIALIZED;
+ }
+
+ // Lock the mutex to protect the global state used by VeraCrypt's format code
+ std::lock_guard<std::mutex> lock(g_sdkMutex);
+
+ finally_do({
+ // Clean up all sensitive data from globals
+ WipePasswordsAndKeyfiles(true);
+ // Reset globals to default state
+ KeyFileRemoveAll(&FirstKeyFile);
+ });
+
+ if (!IsDriveAvailable(static_cast<int>(options->letter))) {
options is dereferenced here before any null check, and options->path is
not validated at all. A null options or null/empty path will crash
instead of returning VCF_ERROR_INVALID_PARAMETER.
Please validate options, options->path, and required password/keyfile
inputs before using any fields.
------------------------------
In src/FormatDLL/VeraCryptFormatSDK.cpp
<#1659 (comment)>:
> + return VCF_ERROR_KEYFILE_ERROR;
+ }
+ mountOptions.DisableMountManager = options->DisableMountManager;
+ StringCbCopyW(mountOptions.Label, sizeof(mountOptions.Label), options->Label);
+ mountOptions.PartitionInInactiveSysEncScope = options->PartitionInInactiveSysEncScope;
+ mountOptions.PreserveTimestamp = options->PreserveTimestamp;
+ mountOptions.ProtectedHidVolPim = options->ProtectedHidVolPim;
+ mountOptions.ProtectedHidVolPkcs5Prf = options->ProtectedHidVolPkcs5Prf;
+ mountOptions.ProtectHiddenVolume = options->ProtectHiddenVolume;
+ mountOptions.ReadOnly = options->ReadOnly;
+ mountOptions.RecoveryMode = options->RecoveryMode;
+ mountOptions.Removable = options->Removable;
+ mountOptions.SkipCachedPasswords = options->SkipCachedPasswords;
+ mountOptions.UseBackupHeader = options->UseBackupHeader;
+
+ int result = MountVolume(NULL, static_cast<int>(options->letter), options->path, &volumePassword,
MountVolume can modify the volumePath buffer it receives. Passing
options->path directly makes the SDK mutate caller-owned input, and the
public struct does not document that requirement.
Please copy options->path into an SDK-owned writable buffer, sized for
TC_MAX_PATH, validate/copy safely, and pass that internal buffer to
MountVolume.
------------------------------
In src/FormatDLL/VeraCryptFormatSDK.cpp
<#1659 (comment)>:
> + }
+ mountOptions.DisableMountManager = options->DisableMountManager;
+ StringCbCopyW(mountOptions.Label, sizeof(mountOptions.Label), options->Label);
+ mountOptions.PartitionInInactiveSysEncScope = options->PartitionInInactiveSysEncScope;
+ mountOptions.PreserveTimestamp = options->PreserveTimestamp;
+ mountOptions.ProtectedHidVolPim = options->ProtectedHidVolPim;
+ mountOptions.ProtectedHidVolPkcs5Prf = options->ProtectedHidVolPkcs5Prf;
+ mountOptions.ProtectHiddenVolume = options->ProtectHiddenVolume;
+ mountOptions.ReadOnly = options->ReadOnly;
+ mountOptions.RecoveryMode = options->RecoveryMode;
+ mountOptions.Removable = options->Removable;
+ mountOptions.SkipCachedPasswords = options->SkipCachedPasswords;
+ mountOptions.UseBackupHeader = options->UseBackupHeader;
+
+ int result = MountVolume(NULL, static_cast<int>(options->letter), options->path, &volumePassword,
+ options->autoDetectEncryptionAlgorithm ? 0 : static_cast<int>(options->encryptionAlgorithm),
This passes an encryption algorithm enum where MountVolume expects the
pkcs5 PRF/hash id. The fifth parameter of MountVolume is int pkcs5, not
an encryption algorithm selector.
The mount API should expose a PRF/hash option, or pass 0 for
autodetection. Encryption algorithm selection is not the right parameter
for this call.
------------------------------
In src/FormatDLL/VeraCryptFormatSDK.cpp
<#1659 (comment)>:
> + mountOptions.ProtectHiddenVolume = options->ProtectHiddenVolume;
+ mountOptions.ReadOnly = options->ReadOnly;
+ mountOptions.RecoveryMode = options->RecoveryMode;
+ mountOptions.Removable = options->Removable;
+ mountOptions.SkipCachedPasswords = options->SkipCachedPasswords;
+ mountOptions.UseBackupHeader = options->UseBackupHeader;
+
+ int result = MountVolume(NULL, static_cast<int>(options->letter), options->path, &volumePassword,
+ options->autoDetectEncryptionAlgorithm ? 0 : static_cast<int>(options->encryptionAlgorithm),
+ options->pim, options->cachePassword, options->cachePim, options->sharedAccess, &mountOptions,
+ TRUE, TRUE
+ );
+ if (options->sharedAccess)
+ return result == 2 ? VCF_SUCCESS : result;
+ return result == 1 ? VCF_SUCCESS : result;
+ }
This maps mount failures to VCF_SUCCESS. MountVolume returns 0 for mount
failure, including wrong password, and VCF_SUCCESS is also 0. As written,
a wrong password can be reported to SDK callers as success.
Please map MountVolume results explicitly: 1 and 2 should map to
VCF_SUCCESS, while 0 and -1 must map to nonzero SDK error codes.
—
Reply to this email directly, view it on GitHub
<#1659 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BBVGRZOPM3EUCPKGFBFML4D4VSDTTAVCNFSM6AAAAACVH6JBPKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHM2DAOJWG43DQNZQGM>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
2 participants
Pull Request: Extend VeraCrypt Format SDK with Mount/Dismount Support and Enum-Based Configuration
Summary
This PR extends the VeraCrypt Format SDK by adding volume mounting and dismounting capabilities, replacing string-based algorithm and filesystem identifiers with enums, introducing size units for container creation, and providing utility functions for path resolution.
Detailed Changes
1. New Enumerations for Type-Safe Configuration
DriveLetter: Defines all possible drive letters (A–Z) for mount operations.VolumeEncryptionAlgorithm: Replaces free-form encryption algorithm strings with a fixed list of supported algorithms (e.g., AES, Serpent, cascades).HashAlgorithm: Replaces hash algorithm strings with an enum covering all supported PRF/Hash functions.FileSystemFormat: Replaces filesystem strings with an enum (NTFS, FAT, ExFAT, ReFS, None).SizeMeasureUnity: Allows specifying container size in KB, MB, or GB instead of raw bytes.Rationale: Using enums eliminates the risk of typos, simplifies validation, and makes the API self-documenting. It also enables better compile-time checks for client applications.
2. Modified
VeraCryptFormatOptionsStructureencryptionAlgorithm,hashAlgorithm, andfilesystemfromconst wchar_t*to the respective enum types.sizeMeasureUnityfield to interpret thesizefield in kilobytes, megabytes, or gigabytes.3. New Mount/Dismount Functionality
VeraCryptMount: Mounts an existing VeraCrypt volume to a specified drive letter.VeraCryptMountOptionsstruct containing password, keyfiles, PIM, mount flags (read-only, hidden volume protection, etc.), and optional hidden volume password.VeraCryptDismount: Unmounts a volume from a given drive letter, with an option to force dismount.GetAbsolutePath: Converts a relative path to an absolute path (useful for resolving keyfile or container paths).GetDevicePath: Retrieves the Windows device path (e.g.,\Device\HarddiskVolumeX) for a mounted drive letter.Rationale: These additions transform the SDK from a volume-creation-only tool into a comprehensive volume management library, enabling applications to fully integrate VeraCrypt operations.
4. Internal Improvements
constexprarrays in the.cppfile to map enum values to the corresponding string identifiers required by the underlying VeraCrypt core.CreateVolumeInternalto use these mappings and to compute the actual byte size based onsizeMeasureUnity.VeraCryptMountby leveraging VeraCrypt’sMountVolumefunction, carefully managing global state and cleaning up sensitive data withfinally_do.VCF_ERROR_FULL_PATH_GETTING_ERROR(24) andVCF_ERROR_DRIVE_LETTER_UNAVIABLE(25).6. Header and Documentation Updates
VeraCryptFormatSDK.hwith detailed comments for new structures and functions..cppfile (<array>,<Shlwapi.h>) to support new functionality.Breaking Changes & Migration Guide
VeraCryptFormatOptions:encryptionAlgorithm,hashAlgorithm,filesystemare now enums. Replace string literals with the corresponding enum values (e.g.,L"AES"→VERACRYPT_VOLUME_ENCRYPTION_AES).sizenow requiressizeMeasureUnityto be set. Previouslysizewas in bytes; now specify the unit separately. For example, to create a 100 MB container, setsize = 100,sizeMeasureUnity = Megabytes.