_Crypt_DecryptData With Blank string?

[Proph]

I am in the process of updating one of my programs to use the new _Crypt_DecryptData instead of the old _StringEncrypt but I noticed that when using _Crypt_DecryptData on a blank string it returns -1.

Which ends up causing issues with some of my code.  I understand that this is probably the intended way for it to work... but it would cause issues if someone were to try to use this function imbedded within other functions... like RegRead.

RegRead returns a blank string if a registry does not exist.  Or for instance IniRead may return a blank string on a key that does not exist.

I feel that _Crypt_DecryptData should be returning blank information if the string is indeed blank. ;-)

I will recode my program to bypass this for now.  Just hoping it can be changed in a future release to simplify coding in the future.  So that I don't have to add extra... if $var = -1 then ....

Proph

[BrewManNH]

You'll notice that  the function returns -1 on an error but it also sets @error, you should be checking for an error and not checking for the return value. The return value, for an error, isn't documented for that function, but the @error values are.

[Proph]

I understand that.  I was just pointing out that the way it handles blank strings... by flagging a blank string as an error... we can not embed the command within other commands like:

_Crypt_DecryptData(RegRead("SomePath", "Key"), "PASSWORD", $CALG_RC4)

 

 

because if the regread does not exist it will return a blank string and _Crypt_DecryptData will then return -1.  Instead I would have to do something like:

$RegVar = RegRead("SomePath", "Key")
If $RegVar <> "" Then
_Crypt_DecryptData($RegVar, "PASSWORD", $CALG_RC4)
EndIf

 

In my opinion it would be simple to change the udf to return a blank string if the original string is blank. Therefore making it much simpler to use. ;-)

Not a big deal... just an observation.

[jchd]

Unfortunately you appear to have chosen the only encryption algorithm which doesn't work with empty input in Crypt.au3.

Something is clearly broken there, but not always:

Local $pass = 'password in use today'
Local $algo = $CALG_RC4
Local $cyphertext = _Crypt_EncryptData(StringToBinary('', 4), $pass, $algo)
_ConsoleWrite("error = " & @error & @LF & _VarDump($cyphertext) & @LF)
Local $res = _Crypt_DecryptData($cyphertext, $pass, $algo)
_ConsoleWrite("error = " & @error & @LF & _VarDump($res) & @LF)
Local $plaintext = BinaryToString($res, 4)
_ConsoleWrite("error = " & @error & @LF & _VarDump($plaintext) & @LF)

; correctly displays:
error = 0
Int32        0
error = 0
Binary       (4) 0xF800AA87
error = 0
String       (4) ''

; but things break down if you change the password (unless you're lucky!)

BTW I never get -1 as return value (v3.3.12.0 or beta 3.3.13.8).

Edited July 24, 2014 by jchd

[BrewManNH]

I understand that.  I was just pointing out that the way it handles blank strings... by flagging a blank string as an error... we can not embed the command within other commands like:

Which I think is the proper way to handle a blank string, you're creating an error condition by trying to decrypt nothing, and the function tells you that.

 

 

because if the regread does not exist it will return a blank string and _Crypt_DecryptData will then return -1.  Instead I would have to do something like:

 

 

$RegVar = RegRead("SomePath", "Key")
If $RegVar <> "" Then
_Crypt_DecryptData($RegVar, "PASSWORD", $CALG_RC4)
EndIf

This would be the correct way to code it. You should never rely on another function's return value if you don't know for sure that it will return what you need every single time, you should always check for an error or unexpected return unless you know 100% that it will/can never fail. Realistically, that's not possible, so check to make sure what you're doing is as bulletproof as you can make it.

 

In my opinion it would be simple to change the udf to return a blank string if the original string is blank. Therefore making it much simpler to use. ;-)

That snippet above makes it so that you don't need to change the UDF, just change your code to expect the unexpected. Edited July 24, 2014 by BrewManNH

[guinness]

I see no issue with the UDF.

[jchd]

The issue is that an empty string plaintext doesn't round trip thru RC4 encryption/decryption. Other available algorithms don't have this issue.

A simple but effective workaround is to prepend pseudo-salt, like this (but it weakens RC4 on bulk use):

 

#include "crypt.au3"

Local $pass = 'Key: select as you see fit'
Local $algo = $CALG_RC4
Local $salt = 'This is salt. Actual plaintext starts here >'
Local $cyphertext = _Crypt_EncryptData(StringToBinary($salt, 4), $pass, $algo)
Local $res = _Crypt_DecryptData($cyphertext, $pass, $algo)
Local $plaintext = BinaryToString($res, 4)
ConsoleWrite($plaintext & @LF)
Local $recovered = StringTrimLeft($plaintext, StringLen($salt))
ConsoleWrite("'" & $recovered) & "'" & @LF)

Edited July 24, 2014 by jchd

[jchd]

All is now clear.

Background:

RC4 is the only cypher (among what Crypt offers) which is nothing more than an XOR of the plaintext bytes and integers in [0, 255] rearranged by the key. Thus if the plaintext has length 0, then the ciphertext also has length 0.

The issue:

_Crypt_EncryptData() and _CryptDecryptData() use DllStructs to hold plaintext and ciphertext and invoke Advapi32.dll functions. In our particular case, the structure length is zero, but DllStructs don't allow for zero-length structure elements:

Local $t = DllStructCreate("byte[0]")
ConsoleWrite("error = " & @error & "  struct size = " & DllStructGetSize($t) & @LF)
Local $read = DllStructGetData($t, 1)
ConsoleWrite("error = " & @error & "  value read is " & VarGetType($read) & " = " & $read & @LF)
Local $read = DllStructGetData($t, Default)
ConsoleWrite("error = " & @error & "  value read is " & VarGetType($read) & " = " & $read & @LF)

; produces

error = 2  struct size = 0
error = 1  value read is Int32 = 0
error = 1  value read is Int32 = 0

From there on, we're trapped with a wrong ciphertext.

So until it's decided how to fix the issue to correctly pick a zero-length variable in this case, please use pseudo-salt.

Edited July 24, 2014 by jchd

[guinness]

Hmm...I have no suggestion as to how or if we should fix this.

[jchd]

If? Yes of course!

How? That is the pending question.

[guinness]

Maybe the fix would come from DllStruct accepting zero byte arrays?

[jchd]

Already asked to Jon >here.

[Melba23]

jchd,

As a stopgap, could we not alter the UDF to look for RC4 encryption of a zero-sized data packet and simply return an empty string - perhaps with @extended set to indicate the fact?

Func _Crypt_EncryptData($vData, $vCryptKey, $iALG_ID, $bFinal = True)
    
    If $vData = "" And $iALG_ID = $CALG_RC4 Then
        Return SetError(0, 1, "")
    EndIf

Or am I being too simplistic?

M23

[jchd]

Yes that's what I've done on my copy. There is little problem due to RC4 being a stream cipher. But this is a ugly kludge and I'm going to bet my first diaper that the zero-length struct item issue will keep on causing problems elsewhere or already have silently.

BTW I see no need to raise extended: everything is as it should be. To be consistent we should return Binary(''), not a string, from both functions.

Edited July 24, 2014 by jchd

[guinness]

It's not a fix I would suggest.

[jchd]

That's not a fix but a garrotte! The actual culprit is elsewhere.

What do you have in mind, short of having the structs fixed themselves?

[Melba23]

guinness,

Why not? It solves the proximate problem immediately and is so simple that it will not be difficult to remove when the actual problem is resolved. Personally I have no problem either way - I do not use RC4 encryption - but I wonder why you seem so against it.

M23

[jchd]

Melba,

I rather see the fix as:

If $iALG_ID = $CALG_RC4 And BinaryLen($vData) = 0 Then Return Binary('')

[Melba23]

jchd,

I bow to your acknowledged superiority in these matters.

M23

P.S. And in English we say "tourniquet" - "garrot" is a method of killing/executing. Although the principle is exactly the same in either case.

Edited July 24, 2014 by Melba23

[jchd]

Superiority, my ass: you fly well above us most of the time!

I used Translate for that word. They mention stopping blood but not Franco routine game.