Xor encryption in C

Question

Based on this code found on github and the advice given to me on stackoverflow I made this code for the xor in c.

    #include <stdio.h>
    #include <string.h>


    void encryptDecrypt(char *input, char *output)
    {
        char key[3] = {'K', 'E', 'Y'};

        for (int i = 0; i < strlen(input); ++i)     

            output[i] = input [i] ^ key [i % sizeof(key)]; 
    }

    int main()
    {
        char baseStr[] = "Test";    

        char encrypted[strlen(baseStr) + 1];
        memset(encrypted, '\0', sizeof(encrypted));
        encryptDecrypt(baseStr, encrypted);
        printf("Encrypted: %s\n", encrypted);

        char decrypted[strlen(baseStr) + 1];
        memset(decrypted, '\0', sizeof(decrypted));
        encryptDecrypt(encrypted, decrypted);
        printf("Decrypted: %s\n", decrypted);

        return 0;

    }

How can I improve the code?

Answer 1

Don't deal in NUL terminated strings. The problem is that there are characters that will "encrypt" to NUL. (In this case, notably, 'K', 'E', and 'Y'.) This means you need to pass in the length of the text.

If you want to allow for future expansion, allow for the possibility that the output text may be longer than the input text. Dynamic allocation may be appropriate, or maybe you have an auxiliary function that says "for a text of length x, what is the longest length of output text." It may also then be appropriate to pass in the output buffer length, and pass out the output used length.

Also allow for the specifying the operation (encrypt/decrypt). This "encryption" doesn't need it, but most symmetric encryptions do.

Answer 2

First, about the existing code:

• key can be initialized with a string literal, "KEY", rather than an array literal - with caveats in the comments
• encryptDecrypt should be static
• It should accept a const char *input as a promise not to modify it
• Since your baseStr is a local array, you do not need to call strlen on it; you can use sizeof

Now, about what the code could be doing:

• Accept the input from stdin or a file instead of having it be hard-coded; similar for the key
• Learn about encryption algorithms that are stronger than this; and learn about the cryptographic weaknesses of xor encryption
• When you print the encrypted string to stdout, do not print its raw string which will be full of unprintable characters. Instead print a hex string.