Crypt32Util.cryptProtectData fails on zero-length array

[dmytro-sheyko]

JNA version: 5.8.0
jna-platform.jar (+ jna.jar)

Java Version: 1.8.0_261
java -version
java version "1.8.0_261"
Java(TM) SE Runtime Environment (build 1.8.0_261-b12)
Java HotSpot(TM) 64-Bit Server VM (build 25.261-b12, mixed mode)

OS Version: Windows 10
Microsoft Windows [Version 10.0.18363.1556]

---

Description of the problem
The method Crypt32Util.cryptProtectData is not able to encrypt zero-length array. Please note that .NET ProtectedData.Protect and direct Windows API CryptProtectData accept and encrypt zero-length data perfectly.

---

Code that reproduces the bug

import com.sun.jna.platform.win32.Crypt32Util;
import com.sun.jna.platform.win32.WinCrypt;

public class JNACryptProtectDataBug {
	public static void main(String[] args) {
		byte[] data = {};
		byte[] encrypted = Crypt32Util.cryptProtectData(data, WinCrypt.CRYPTPROTECT_UI_FORBIDDEN);
		System.out.println("ok");
	}
}

Expected

ok

Actual

Exception in thread "main" java.lang.IllegalArgumentException: Allocation size must be greater than zero
	at com.sun.jna.Memory.<init>(Memory.java:239)
	at com.sun.jna.platform.win32.WinCrypt$DATA_BLOB.<init>(WinCrypt.java:75)
	at com.sun.jna.platform.win32.Crypt32Util.cryptProtectData(Crypt32Util.java:80)
	at com.sun.jna.platform.win32.Crypt32Util.cryptProtectData(Crypt32Util.java:60)
	at JNACryptProtectDataBug.main(JNACryptProtectDataBug.java:7)

[dbwiddis]

The API isn't really clear on whether a null pointer or empty array is permitted, and it seems nonsensical to encrypt an empty array. What's the appropriate return value? It's undocumented, and potentially undefined.

Seems this could be fixed by length-checking data and returning an empty byte[] array at Crypt32Util.java line 80.

jna/contrib/platform/src/com/sun/jna/platform/win32/Crypt32Util.java

Lines 78 to 80 in 5ac6161

	public static byte[] cryptProtectData(byte[] data, byte[] entropy, int flags,
	String description, CRYPTPROTECT_PROMPTSTRUCT prompt) {
	DATA_BLOB pDataIn = new DATA_BLOB(data);

Or we could alter DATA_BLOB to accept 0-length input, leaving the pointer null.

jna/contrib/platform/src/com/sun/jna/platform/win32/WinCrypt.java

Lines 74 to 76 in 5ac6161

	public DATA_BLOB(byte [] data) {
	pbData = new Memory(data.length);
	pbData.write(0, data, 0, data.length);

It could also be fixed in user code by length-checking before calling what amounts to a no-op method.

[matthiasblaesing]

I think the variant to create a DATA_BLOB for a 0-length array is the way to go. DATA_BLOB#getData is already prepared to handle a null pointer for pbData. It would be trivial to rewrite the byte[] constructor to (untested!):

        public DATA_BLOB(byte [] data) {
            if(data == null || data.length == 0) {
                pbData = null;
                cbData = 0;
            } else {
                pbData = new Memory(data.length);
                pbData.write(0, data, 0, data.length);
                cbData = data.length;
            }
            allocateMemory();
        }

What the w32 API will to with that is a totally different matter and also needs testing.

[dmytro-sheyko]

@matthiasblaesing, thanks for suggestion, I will try to test it. But I would note that 1-byte array is still ok it does not affect the final result because we pass the size of data in cbData anyway. So the buffer in pbData can be larger if necessary.

[dmytro-sheyko]

@matthiasblaesing, unfortunately the approach with pbData = null fails.

com.sun.jna.platform.win32.Win32Exception: The parameter is incorrect.
	at com.sun.jna.platform.win32.Crypt32Util.cryptProtectData(Crypt32Util.java:86)
	at com.sun.jna.platform.win32.Crypt32Util.cryptProtectData(Crypt32Util.java:60)
	at com.sun.jna.platform.win32.Crypt32Util.cryptProtectData(Crypt32Util.java:47)
	at com.sun.jna.platform.win32.Crypt32UtilTest.testCryptProtectUnprotectZeroLengthData(Crypt32UtilTest.java:41)

[matthiasblaesing]

Welcome to the wonderful world of C. I assume, from the documentation:

malloc() allocates size bytes and returns a pointer to the allocated memory. The memory is not cleared. If size is 0, then malloc() returns either NULL, or a unique pointer value that can later be successfully passed to free().

that the second variant of malloc is expected. Crap - it would be good to check on windows: What pointer does malloc(0) return? I suspect it is not a null pointer, else C# code I found online:

        // Allocate memory for the BLOB data.
        blob.pbData = Marshal.AllocHGlobal(data.Length);
 
        // Make sure that memory allocation was successful.
        if (blob.pbData == IntPtr.Zero)
            throw new Exception(
                "Unable to allocate data buffer for BLOB structure.");Marshal.AllocHGlobal(data.Length);

would fail (assuming AllocHGlobal uses malloc and not some internal magic.

[matthiasblaesing]

In any case - it needs to be documentation above the Memory allocation why a 1 byte allocation is done even if the byte[] has zero length.

[dmytro-sheyko]

corrected pull request: added comment explaining why we allocate 1 byte instead of 0.