Extra Characters printed in the file

Question

I have made a program to encrypt and decrypt a message using a vigenere cipher.

While encrypting or decrypting the text, some extra garbage values are printed in it.

It takes the input from a file named input.txt and outputs to output.txt, you have to write the message in the input.txt file and while running you have to give a key (a word with alphanumeric characters).

Why is this is happening?

The code is as follows:

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



int Encrypt(char key[])         // CODE FOR ENCRYPTION
{
    int sz = 0, i;
    FILE *ifp, *ofp;
    ifp = fopen("input.txt", "r");
    char *buffer;
    char outputFilename[] = "output.txt";


    if (ifp == NULL)
    {
        fprintf(stderr, "Cant open input file\n");
        exit(1);
    }


    fseek(ifp, 0, SEEK_END);
    sz = ftell(ifp);

    // printf("%d",sz);

    fseek(ifp, 0, SEEK_SET);


    /* allocate memory for entire content */
    buffer = (char *)malloc(sizeof(char) * sz);
    if (!buffer)
        fclose(ifp), fputs("memory alloc fails", stderr), exit(1);

    /* copy the file into the buffer */
    if (1 != fread(buffer, sz, 1, ifp))
        fclose(ifp), free(buffer), fputs("entire read fails", stderr), exit(1);


    ofp = fopen(outputFilename, "w");

    if (ofp == NULL)
    {
        fprintf(stderr, "Can't open output file !\n");
    }
    // fprintf(ofp,"%s",buffer);

    int j = 0;
    for (i = 0; i < strlen(buffer); i++)
    {
        if (j > strlen(key) - 1)
            j = 0;
        if (buffer[i] >= 65 && buffer[i] < 91)
        {

            int c = ((((buffer[i] - 65) + ((key[j] - 65) % 26))) % 26) + 65;
            fprintf(ofp, "%c", c);

        }
        else if (buffer[i] >= 97 && buffer[i] < 123)
        {
            int c = ((((buffer[i] - 97) + ((key[j] - 65) % 26))) % 26) + 97;

            fprintf(ofp, "%c", toupper(c));

        }
        else
        {
            fprintf(ofp, "%c", buffer[i]);
            continue;
        }
        j++;
    }
    printf("\n");

    fclose(ifp);
    fclose(ofp);

    return 0;
}

int Decrypt(char key[])         // CODE FOR DECRYPTION
{

    int sz = 0, i;
    FILE *ifp, *ofp;
    ifp = fopen("output.txt", "r");
    char *buffer;
    char outputFilename[] = "output2.txt";


    if (ifp == NULL)
    {
        fprintf(stderr, "Cant open input file\n");
        exit(1);
    }


    fseek(ifp, 0, SEEK_END);
    sz = ftell(ifp);

    // printf("%d",sz);

    fseek(ifp, 0, SEEK_SET);

    /* allocate memory for entire content */
    buffer = (char *)malloc(sizeof(char) * sz);
    if (!buffer)
        fclose(ifp), fputs("memory alloc fails", stderr), exit(1);

    /* copy the file into the buffer */
    if (1 != fread(buffer, sz, 1, ifp))
        fclose(ifp), free(buffer), fputs("entire read fails", stderr), exit(1);


    ofp = fopen(outputFilename, "w");

    if (ofp == NULL)
    {
        fprintf(stderr, "Can't open output file !\n");
    }
    // fprintf(ofp,"%s",buffer);

    int j = 0;
    for (i = 0; i < strlen(buffer); i++)
    {
        if (j > strlen(key) - 1)
            j = 0;
        if (buffer[i] >= 65 && buffer[i] < 91)
        {
            if (buffer[i] > key[j])
            {
                int c =
                    ((((buffer[i] - 65) - ((key[j] - 65) % 26))) % 26) + 65;
                fprintf(ofp, "%c", tolower(c));
            }
            else
            {
                int c = ((((buffer[i] - key[j]) + 26)) % 26) + 65;
                fprintf(ofp, "%c", tolower(c));
            }
        }
        else if (buffer[i] >= 97 && buffer[i] < 123)
        {
            int c = ((((buffer[i] - 97) - ((key[j] - 65) % 26))) % 26) + 97;

            fprintf(ofp, "%c", tolower(c));

        }
        else
        {
            fprintf(ofp, "%c", buffer[i]);
            continue;
        }
        j++;
    }
    printf("\n");

    fclose(ifp);
    fclose(ofp);

    return 0;
}

void main()
{
    int ch;
    char key[20];
  a:printf("0.Exit the Menu\n1.Encrypt\n2.Decrypt\n");
    printf("Enter your choice\n");
    scanf("%d", &ch);


    switch (ch)
    {

    case 0:
        printf("Goodbye\n");
        break;

    case 1:
        printf
            ("-----------------------------Welcome to the encryption zone---------------------\n");
        printf("Enter the key to be used\n");
        scanf("%s", key);

        Encrypt(key);
        break;

    case 2:
        printf
            ("-----------------------------Welcome to the decryption zone---------------------\n");
        printf("Enter the key to be used\n");
        scanf("%s", key);

        Decrypt(key);
        break;

    default:
        printf("Enter the correct choice\n");
        goto a;
        break;

    }
}

Answer 1

When you are allocating and copying here

buffer = (char *)malloc(sizeof(char) * sz);
if (!buffer)
    fclose(ifp), fputs("memory alloc fails", stderr), exit(1);   

/* copy the file into the buffer */ 
if (1 != fread(buffer, sz, 1, ifp))
    fclose(ifp), free(buffer), fputs("entire read fails", stderr), exit(1);

you've allocated sz bytes in buffer, and copied sz bytes, but you didn't leave room for the null terminator. Hence, when you check strlen(buffer) and encrypt later, you're going off into memory you didn't allocate.

To fix this, either you have to allocate one extra byte for the '\0', or you copy over one less and tack on the '\0' at the end.

Answer 2

Wrong length calculation.

As mention by @Dennis Meng, the strlen(buffer) likely (*1) goes off past the end of your allocated buffer. Your malloc() is fine. The recommended solution is different. Rather than tacking on a NUL char, change for 2 for loops

for (i = 0; i < strlen(buffer); i++)

to

for (i = 0; i < sz; i++)

Not only will this fix the problem, it will run faster as you are no longer performing strlen(buffer) sz times.

*1 It would not go that far had your file contained a NUL character. If it does go past the end of buffer, where things stop is UB.