MaxLo MaxLo - 1 month ago 10
C Question

What is wrong with this C code to encode/decode messages

Our code is supposed to have two files. code.c that encodes and decode.c that decodes from stdin. It shouldn't have an issue with any length file sizes, assuming that each line is 120 characters or less.

My encode code...

void encode(char pass[])
{

char buffer[121], encoded[121];
int i=0, j=0;

while( fgets(buffer, 120, stdin) )
{

for(i=0; i<(strlen(buffer)); i++)
{
if(j>(strlen(pass)-1))
j=0;


encoded[i] = buffer[i] + pass[j];
j++;
}

fprintf(stdout, encoded);

}
fprintf(stderr, buffer);
}


And the exact opposite, the decode code:

void decode(char pass[])
{

char buffer[121], decoded[121];
int i=0, j=0;

while( fgets(buffer, 120, stdin) )
{

for(i=0; i<(strlen(buffer)); i++)
{
if(j>(strlen(pass)-1))
j=0;

decoded[i] = buffer[i] - pass[j];
j++;
}

fprintf(stdout, decoded);
}


}


The code seems to execute and work almost as intended, except that it seems to encode more data on the last line when I do CTRL+D to terminate. So when I go to decode, it decodes properly except for the last line.

Edit:

here is some sample input

code password > crypt
This is a test
file that will contain
encrypted information


That should put

This is a test
file that will contain
encrypted information


into a file named crypt with the password of 'password'

The file crypt looks like this:

ÄÉÜæ<97>Øå<84>Ñ<81>çØêãÊÙÍØ<93>ë×ÓØ<90>ØÜßã<8f>ÕÓÞÕÔÜåy×ÒÓÓìãëÔÖ<84>ÙÏÙâéÜÓØÙÐá}y


Decryption output looks like this however:

decode password < crypt
This is a test
file that will contain
encrypted information
??


The ?? is garbage text that it's pulling out of the file. Not sure why its either encoding or decoding more than was input into stdin..

Another round of input/output does this:

./code password > crypt
testing this
file to encrypt
stuff
./decode password < crypt
testing this
file to encrypt
!J


Whereas it's much more clear that there are issues happening with my last line.

Answer

The main problem is you do not put a null terminator at the end of encoded or decoded before passing it to fprintf().

Note however that you should not pass decoded as a format string to fprintf, it would invoke undefined behavior if it were to contain % characters.

Here is a corrected and simplified version:

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

void encode(const char pass[]) {
    int c, i = 0, len = strlen(pass);

    while ((c = getchar()) != EOF) {
        putchar(c + pass[i++]);
        if (i >= len)
            i = 0;
    }
    fflush(stdout);
}

void decode(const char pass[]) {
    int c, i = 0, len = strlen(pass);

    while ((c = getchar()) != EOF) {
        putchar(c - pass[i++]);
        if (i >= len)
            i = 0;
    }
    fflush(stdout);
}

If you want to keep reading input one line at a line, here is a simplified version of your code that does not copy the string to a different buffer and therefore does not need to store the null terminator:

void encode(char pass[]) {
    char buffer[121];
    int i = 0, j = 0, len = strlen(pass);

    while (fgets(buffer, sizeof buffer, stdin)) {
        for (i = 0; buffer[i] != '\0'; i++) {
            buffer[i] += pass[j++];
            if (j >= len)
                j = 0;
        }
        printf("%s", buffer);
    }
}

Note however that this method fails if the input file contains null bytes or characters such that adding a byte from the password produces the value 0. It is impossible if both the file and the password contains only ASCII bytes, but if you try and encode a binary file or a foreign text file, you may run into this issue. The simpler version with getchar() does not have this problem.