samuel-lucas6 / Kryptor

A simple, modern, and secure encryption and signing tool that aims to be a better version of age and Minisign.
https://www.kryptor.co.uk
GNU General Public License v3.0
414 stars 33 forks source link

🐛 Bug: Decrypting a folder - Zero length files aren't decrypted #27

Closed highend closed 2 years ago

highend commented 2 years ago

Description

I've just wanted to try out kryptor so I've just encrypted a folder with some files in it. Quite a few of them do not have any content and aren't saved with a BOM (regardless of their extension, most are just text files) so they have a file size of 0 bytes.

Steps to Reproduce

kryptor.exe -e -o -y "D:\gf.public" "R:\tst\Temp" All files are encrypted successfully, including the 0-bytes files

kryptor.exe -d -o -y "D:\gf.public" "R:\tst\Temp"

test.py.kryptor => test.py
y.jpg.kryptor: Non-negative number required. (Parameter 'value')
y.txt.kryptor: Non-negative number required. (Parameter 'value')
z.abc.kryptor: Non-negative number required. (Parameter 'value')
z.ahk.kryptor: Non-negative number required. (Parameter 'value')
z.bmp.kryptor: Non-negative number required. (Parameter 'value')
z.eml.kryptor => z.eml
etc.

The original test.py and z.eml are not 0-byte files, the others are

Expected Behaviour

Even 0-byte files should be decrypted. Otherwise you have a lot of .kryptor files lying around and e.g. a lot of portable applications on Windows use a e.g. portable.dat file in their root folder that is empty and let them know (if it exists) that they should run in portable mode. The current situation will break that behavior!

Platform Info

samuel-lucas6 commented 2 years ago

Hi @highend,

Many thanks for the report. It unfortunately looks like I never tested encrypting/decrypting 0 byte files properly.

I just tried encrypting and decrypting an empty text file, and it did decrypt to a 0 byte file, but I also got that error message, and the .kryptor file was not deleted because of the exception. I can already see the problematic line in the code. The program tries to set the length of the file to less than 0 because there's no padding to remove.

If you use -f|--obfuscate, then there's no error message because there's a chunk after the headers. However, I should warn you that doing this will cause the 0 byte files to increase in size to 16 KiB once encrypted because Kryptor is designed to hide the exact length of the file. I have mixed feelings about whether that's the right approach to take since it increases storage requirements and other programs don't do that. Please let me know what you think. It would require a breaking change to tweak though.

Anyway, I'll get this fixed for the next release. I'm afraid I'm going on holiday, so it'll take some time. There are also a few other minor things I'd like to add for v3.0.4 that I haven't managed to do yet because I've been working on some other projects.

highend commented 2 years ago

Hi,

It's a design decision and it's yours :) I guess any file that only has a few bytes will be enlarged to at least 16 KiB with -f|--obfuscate so it doesn't matter if it does it for 0-byte files as well. I wouldn't want to get notifications on the command line when something like this happens, at least it would be nice if this could be suppressed. Take your time to fix this and have a nice holiday!

samuel-lucas6 commented 2 years ago

Thanks! I think that's best for now at least. I'll perhaps remove the chunking for small files in v4. We'll see.