C basic programming concept

Question

I am new to C programming and I am writing a program to do 3DES encryption.

But there are some fundamental mistakes in this code such as doing malloc within the function and not deallocating. Could someone help me in rewriting this by using a global variable and then deallocating? I want to optimize this code.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <openssl/des.h>
#include <openssl/rand.h>

#define BUFSIZE 128 


char *
Encrypt( char *Key, char *Msg, int size)
{

        static char*    Res;
        unsigned char in[BUFSIZE], out[BUFSIZE], back[BUFSIZE];
        unsigned char *e = out;
        char buffer[21]="";
        char *pbuffer = buffer;
        int len;

        DES_cblock key1, key2, key3;
        DES_cblock seed = {0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10};
        DES_cblock ivsetup = {0xE1, 0xE2, 0xE3, 0xD4, 0xD5, 0xC6, 0xC7, 0xA8};
        DES_cblock ivec;
        DES_key_schedule ks1, ks2, ks3;

        memset(in, 0, sizeof(in));
        memset(out, 0, sizeof(out));
        memset(back, 0, sizeof(back));

        DES_string_to_key (Key, &key1);
        DES_string_to_key (Key, &key2);
        DES_string_to_key (Key, &key3);

        DES_set_key((C_Block *)key1, &ks1);
        DES_set_key((C_Block *)key2, &ks2);
        DES_set_key((C_Block *)key3, &ks3);

        strcpy(in, Msg);

        //printf("In Encrypt, Plaintext: [%s]\n", in);

        len = strlen(in);
        memcpy(ivec, ivsetup, sizeof(ivsetup));
        DES_ede3_cbc_encrypt(in, out, len, &ks1, &ks2, &ks3, &ivec, DES_ENCRYPT);

        //printf("In Encrypt, Ciphertext:");
        while (*e) 
        {
            //printf("%02x", *e);
            sprintf(pbuffer, "%02x", *e);
            pbuffer +=2;
            *e++;
        }
        //printf("\n");
        //printf("In Encrypt, Returning Text: [%s]\n", buffer);

        Res = ( char * ) malloc(sizeof(buffer));
        memcpy(Res, buffer, sizeof(buffer));
        return(( unsigned char * )Res);
}

char *
Decrypt( char *Key, char *Msg, int size)
{

        static char*    Res;

        unsigned char in[BUFSIZE], out[BUFSIZE], back[BUFSIZE];
        unsigned char *e = out;
        char buffer[21] = "";
        char *pbuffer = buffer;
        int len;

        DES_cblock key1, key2, key3;
        DES_cblock seed = {0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10};
        DES_cblock ivsetup = {0xE1, 0xE2, 0xE3, 0xD4, 0xD5, 0xC6, 0xC7, 0xA8};
        DES_cblock ivec;
        DES_key_schedule ks1, ks2, ks3;

        memset(in, 0, sizeof(in));
        memset(out, 0, sizeof(out));
        memset(back, 0, sizeof(back));

        DES_string_to_key (Key, &key1);
        DES_string_to_key (Key, &key2);
        DES_string_to_key (Key, &key3);

        DES_set_key((C_Block *)key1, &ks1);
        DES_set_key((C_Block *)key2, &ks2);
        DES_set_key((C_Block *)key3, &ks3);

        strcpy(in, Msg);

        //printf("In Decrypt, Plaintext: [%s]\n", in);

        len = strlen(in);
        memcpy(ivec, ivsetup, sizeof(ivsetup));
        DES_ede3_cbc_encrypt(in, out, len, &ks1, &ks2, &ks3, &ivec, DES_DECRYPT);

        //printf("In Decrypt, Ciphertext:");
        while (*e) 
        {
            //printf("%02x", *e);
            sprintf(pbuffer, "%02x", *e);
            pbuffer +=2;
            *e++;
        }
        //printf("\n");
        //printf("In Decrypt, Returning Text: [%s]\n", buffer);

        Res = ( char * ) malloc(sizeof(buffer));
        memcpy(Res, buffer, sizeof(buffer));
        return(( unsigned char * )Res);
}

int main(void)
{
    char key[]="1234567890123456"; // 16
    char clear[]="Arun Das";
    char *decrypted;
    char *encrypted;

    printf("In Main, Plain text\t : %s \n",clear);
    encrypted=Encrypt(key,clear,sizeof(clear));
    decrypted=Decrypt(key,encrypted,sizeof(encrypted)); 
    printf("In Main, Encrypted text\t : %s \n",encrypted);
    printf("In Main, Decrypted text\t : %s \n",decrypted);
    system("PAUSE");
    exit(0);
}

Answer 1

Don't use a global variable - instead use free() on returned pointers once the calling code (main()) is done with the results.

Your implementation is quite good - both functions allocate memory and pass ownership of it to the calling code. The calling code becomes responsible for freeing that memory. This is concise and clear. Introducing a global variable will make it worse.

Think of it this way - malloc() does the same (except it allocates memory and doesn't fill it). How could malloc() be done clearer with a global variable accessible to the calling code?