DLL export procedures for encrypting and decrypting messages aren't handling spaces properly

[maroonbells]

I'm writing this from the perspective of using this DLL in AdiIRC, and this could also be a way to enable older mIRC versions to use some-but-not-all of the FiSH functionality. i.e. the older mIRC could encrypt and decrypt messages to a limited degree, but only by trapping ON INPUT and ON TEXT.

Since AdiIRC can't use the internal hooks designed to interface with mIRC, it instead uses the ON INPUT/PARSELINE events to monitor incoming and outgoing messages, and uses DLL external procedures to perform the encryption and decryption. It uses 'FiSH_GetKey10' to get the channel key, it then uses 'FiSH_encrypt_msg' which uses that returned key-string to encrypt the outgoing message, or uses 'FiSH_decrypt_msg' to decrypt incoming messages. However, when trying to use DLL export functions to handle the encryption and decryption, there are some bugs.

(1) Message with leading space have those spaces transferred to be part of the key. This problem doesn't happen when FiSH automatically intercepts the incoming/outgoing messages, but only when using the "FiSH_encrypt_msg" procedure in $dll().

When a %message containing a leading space is used as input into "FiSH_encrypt_msg", the message decrypts to garbage because any extra spaces at the beginning of the message are being included as part of the key instead of being part of the message being encrypted. The next command shows that %msg can be correctly decrypted by FiSH itself using the "FiSH_decrypt_msg" procedure.

//var -s %key cbc:key , %msg $+($chr(72),testmsg) , %enc $dll(FiSH_10.dll, FiSH_encrypt_msg, %key %msg) | echo -a this should work: $dll(FiSH_10.dll, FiSH_decrypt_msg, %key $remove(%enc,)) | echo -a this should fail: $dll(FiSH_10.dll, FiSH_decrypt_msg, $+(%key,$chr(32)) $remove(%enc,))

The 1st decryption uses the correct key, and decrypts the message correctly, but the 2nd decryption appends an extra space to the end of the key, and the decryption is garbled due to using the wrong key.

However, when you edit the above 72 into 32 which causes the message to begin with a space instead of 'H', the 1st decryption using the correct key is garbled, but the 2nd decryption using the extra space in the key is able to decrypt the message.

This means that the "FiSH_decrypt_msg" procedure can only decrypt the message by correctly guessing how many extra spaces should be inserted between the 'key' and the mime string, in order to replicate the space-padded key actually used to encrypt that message differently than how the other messages were encrypted.

(2) Keys containing space(s) use only the 1st 'word' as the key, and the remaining 'words' of the key are shifted into being part of the message being encrypted. This problem doesn't happen when FiSH automatically intercepts the incoming/outgoing messages, but only when using the export procedures to encrypt and decrypt the message, such as using "FiSH_encrypt_msg".

The fish_10.mrc script permits creating keys containing spaces. And this command typed in the channel's editbox:

//echo -a $dll(%FiSH_dll,FiSH_GetKey10, $network $chan)

... reported back to me the same string I used to create the key:

cbc:a very long and hopefully strong passphrase

This is not compatible with "FiSH_encrypt_msg" and "FiSH_decrypt_msg", whose syntax assumes the 1st space-delimited token is the entire key, and the remainder of the string is either the message being encrypted or is a no-spaces encrypted string needing to be decrypted. When the above example of a multi-word key is set, "FiSH_encrypt_msg" mis-interprets the key. Instead of encrypting the short message using the long key, the actual key used is the single letter 'a', and the remainder of the key is moved into being part of the message. It's demonstrated by "FiSH_decrypt_msg" being able to decode the message using only 'cbc:a' as the key:

//var -s %key cbc:a very long and hopefully strong passphrase , %enc $dll(FiSH_10.dll, FiSH_encrypt_msg, %key message) | echo -a this fails: $dll(FiSH_10.dll, FiSH_decrypt_msg, %key $remove(%enc,)) | echo -a this decrypts msg containing most of the key: $dll(FiSH_10.dll, FiSH_decrypt_msg, cbc:a $remove(%enc,))

This isn't much of a security hole, because even though your own script tells you that you used the correct key to encrypt the correct string, nobody else in channel is able to see you blurting out most of the key at the front of each of your messages. They would either see your messages being garbled due to being encrypted using 'cbc:a', or would be unable to decrypt a message since spaces are invalid within a string they expect to be an encoded message.

(3) ANSI vs UTF8 key. 'FiSH_GetKey10' can't be used to 100% obtain the correct key. Also, the "decode_utf8" style of creating keys is defective, but that's reported in issue#68.

When using the menu to create the channel key, the 2 menu choices differ in whether they call the 'FiSH.WriteKey' alias having the $1 parameter be either 'decode_utf8' or 'raw_bytes'.

This means that, when setting the key to be the 4 char string tést, depending on which menu choice you use to set the key, blow.ini shows the key as being either:

menu choice "set new key (UTF-8)"
key=+OK gUp3Q.odIbx1
decrypts as hex bytes: 74 c3 a9 73 74 00 00 00

menu choice "set new key"
key=+OK L7cOo..W0qH.
decrypts as hex bytes: 74 e9 73 74 00 00 00 00

Since FiSH doesn't permit 0x00 to be part of the key, these keys are shortened to the 4 or 5 length strings.

When using $dll(FiSH_10.dll,FiSH_GetKey10,$network $chan) to display the key or use it as input for the "FiSH_encrypt_msg" procedure, mIRC always UTF8-encodes the string returned from the DLL, regardless how the key was created. Even though these are 2 different keys, when a script obtains the %key from the "FiSH_GetKey10" procedure as an input to the "FiSH_encrypt_msg" procedure, it always sees the same string, regardless of the actual key, so the only thing which determines which key is being used for creating the message is whether the script chooses to use 'raw_bytes' as a parameter of FiSH_EncryptMsg10, or uses FiSH_encrypt_msg which defaults to using 'decode_utf8'.

Even though these key strings are very similar, the resulting encryption appears no more related to each other than when compared to other vastly different keys.

==

This behavior is happening in both procedures "FiSH_EncryptMsg10" and "FiSH_encrypt_msg" and the related "FiSH_DecryptMsg10" and "FiSH_decrypt_msg", with the difference between them being that the *Msg10's require that "decode_utf8" or "raw_bytes" are the 1st parameter inserted in front of the key, while the others always insert "decode_utf8" in front of the 'data' string.

These issues are caused by the fact that, when a DLL is called by $dll(filename,procedure,data), the 'data' is a single string that gets passed to it, so it's forced to make assumptions about the string.

The 1st assumption about the input string is that the key is exactly 1 'word' preceding the message.

The 2nd assumption is that when the 1-word-key is parsed apart from the message following it, if there's more than 1 space between them, the extra spaces are assigned as being appended to the key instead of being inserted at the front of the following string.

Solving issue #1 would require changing how the key and message are parsed from each other. I'm assuming the problem is caused by

const string_vector l_data = SplitString(a_data, " ", 3);

... but I don't know the way to make the extra space(s) be inserted into the following token (message) instead of appended to the preceding token (key).

Issues #2 and #3 could be solved by allowing a 3rd choice for the inserted word required by FiSH_EncryptMsg10. In addition to the current "raw_bytes" or "decode_utf8" keywords, it could also allow the +OK prefix contained in blow.ini (or some other keyword), which would allow it to see the key as being encoded in the same Fish64 encoding within blow.ini when the line begins with 'key=+OK'. This encoding allows the key to always be 1 word without any spaces, yet permits the DLL to use the true key inside the encoding, regardless whether it contains embedded spaces, and regardless whether it's in UTF8 or ANSI format.

Adding an .ini item= to indicate whether the key was created using raw_bytes or decode_utf8 would solve #3 only, by providing guidance for a script to know whether to feed the key to "FiSH_EncryptMsg10" using "raw_bytes" or "decode_utf8". As things exist currently, FiSH_GetKey10 doesn't inform a script whether the key is ANSI or UTF8, because even if the DLL sends the ANSI string to mIRC, the string always gets UTF8 encoded before the script can see it. The only way for a script to know if a key was created using raw_bytes or decode_utf8 - is to use FiSH_WriteKey10 to write the key in both formats as if they belong to 2 dummy channels, then use $readini to see which encoded key matches that of the original channel's key.

//echo -a $dll(%FiSH_dll,FiSH_WriteKey10,raw_bytes dummy #raw_bytes tést)
//echo -a $dll(%FiSH_dll,FiSH_WriteKey10,decode_utf8 dummy #decode_utf8 tést)

The 'FiSH_GetKey10' might need an additional parameter to indicate whether to return the key instead as hex or mime, instead of returning a text string that mIRC may or may not change when it translates it into a text string.

If this '+OK' prefix option for using the encoded key is added, it would be great for FiSH to also have a new option to stop encrypting the key being stored inside blow.ini, and just store it in normal mime. This encrypted-fish64 obfuscation would add extra overhead to encrypting and decrypting each message without any real concrete improvement in security. Even if someone hexedited the DLL to change that default obfuscating password 'blowinikey', the password can be obtained by using alias FiSH.showkey to see it. By just encoding the key as mime, that would make it easier for someone to inspect their own keys using $decode.