XOR Encryption

[gooseantlers]

This is one of my first real programs that does anything. I have it pieced together and it is a rough model at the moment. It isn't user friendly yet so bear with me. The file you are encrypting must exist before hand. When I Run Decrypt it Decrypts the first 30 or so characters but then the rest is Jibberish. Please be kind I'm beginning. I also found out having a decrypt function was unnecessary because I can reuse the encrypt.

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

void filename_accept(char fileName[]);
void firstpass_encrypt(char string[], char key[],int size);
//void firstpass_decrypt(char key[], char string[], int size);

int position;
int main()
{
    FILE *myfile;
	FILE *encryptedfile;
	char fileName[50];
	char userchoice[10];
	char key[63]="ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789";
	char c;
	int i = 0;

	startagain: ;
	filename_accept(fileName);
    myfile = fopen(fileName,"r");

	fseek(myfile, 0L, SEEK_END);
	int size = ftell(myfile);

	int linelen = size;
	char*string = (char*)malloc(size*sizeof(string));
	if (string == NULL)
		{
			printf("\nMemory Overflow");
			exit(-1);
		}
    /*printf("This is the length %i", size);*/
	fseek(myfile, 0L, SEEK_SET);

	while((c = fgetc(myfile)) != EOF)
		string[i++] = c;

		BackToTop: ;
		printf("The file we are working with is ");
		printf(fileName);
		printf("\nAll input is case sensitive until next version!\n");
		printf("\nWould you like to Encrypt or Decrypt?  \nPlease type one of the following: \n 1 Encrypt for Encrypt \n 2 Decrypt for Decrypt\n 3 Quit to terminate program ");
		printf("\n 4 Type again to open a different file.\n");
			fflush(stdin);
			if (fgets(userchoice, sizeof userchoice, stdin) != NULL);
			{
				char *newline = strchr(userchoice, '\n');   //search for newline
				if (newline != NULL)
					*newline = '\0';         //overwrite  newline
			}

			int strcheck1 = strcmp(userchoice, "Encrypt");
			int strcheck2 = strcmp(userchoice, "Decrypt");
			int strcheck3 = strcmp(userchoice, "quit");
			int strcheck4 = strcmp(userchoice, "Again");

			if(strcheck1 == 0)
			{
				firstpass_encrypt(string, key, size);
    			/*printf("%c", string[position]);*/
            	strcat(userchoice, ".txt");                        //adds .txt format to it
				encryptedfile = fopen(userchoice, "w");
				for(int x=0; x <= size; x++)
					fprintf(encryptedfile, "%c", string[x]);
				printf("\nYour file has been encrypted.");
				fclose(encryptedfile);
			}
			else if(strcheck2 == 0)
			{
				firstpass_encrypt(key, string, size);
				strcat(userchoice, ".txt");
				encryptedfile = fopen(userchoice, "w");
				for (int x=0; x <= size; x++)
					fprintf(encryptedfile, "%c", string[x]);
				printf("\nYour file has been decrypted");
				fclose(encryptedfile);
			}
			else if(strcheck3 == 0)
				return 0;

			else if(strcheck4 == 0)
				goto startagain;

			else
			{
				printf("%i", strcheck1);
				printf("%i", strcheck2);
				printf("\nInvalid Input.  Please follow the specified format");
				goto BackToTop;
			}



	//printf("\n\n%i", position);

	free(string);
	fclose(myfile);
	fclose(encryptedfile);
	system("pause");
    return 0;
}


void filename_accept(char fileName[])    /*Reads in file name*/
{
	printf("\nEnter file you would like to open:\n");
	gets(fileName);
}

void firstpass_encrypt(char string[], char key[],int size)
{
	for(position=0; position < size; position++)
		{
			if(string[position] == '\0')
				break;
			else
			string[position]=string[position]^key[position%64];
		}
}

/*void firstpass_decrypt(char key[], char string[], int size)
{
	for(position = 0; position < size;position++)
		{
			if (string[position] == '\0')
				break;
			else
				string[position]=key[position%64]^string[position];
		}
}*/

[Subsonics]

Your key seems to only have 62 characters, yet you use modulus 64 to find an index in the key.

[gooseantlers]

I changed the modulus and it still decrypts the first 30 or so characters and then the jibberish remains.

[memcpy]

SourceForge.net: Gets - cpwiki
SourceForge.net: Fflush - cpwiki
SourceForge.net: Delay - cpwiki

[quzah]

What happens if you xor 'a' with 'a'?


Quzah.

[whiteflags]

gets(), gotos and fflush(stdin); ...

My 2 cents: You need to stop relying on things that are easy like this.

• FAQ > Why gets() is bad / Buffer Overflows - Cprogramming.com
• FAQ > Why fflush(stdin) is wrong - Cprogramming.com
• For, While and Do While Loops in C - Cprogramming.com

How is it that you know how to write functions yet you rely on goto all the time to control the flow of the program? You should be using loops and functions more, even if you think it's trivial.

Also, implement a one time pad if you really want to keep it simple and a secret...

edit - Well damn.

[gooseantlers]

memcpy. I feel like you are driving at a point. The Fflush was probably unnecessary. I'm guessing.

[Subsonics]

I changed the modulus and it still decrypts the first 30 or so characters and then the jibberish remains.

Another possibility is that the encrypted file contains a '\0' character, you have set up your decryption function to break at '\0'.

[gooseantlers]

whiteflags. What do you mean one time pad? I guess I just found it very simple to do the goto at the moment. As to why I know how to write functions and don't use them enough. I just wrote the program continuously through and was going to begin chopping the program up into functions by identifying what could be reused in the functions. Thanks for the advice. Would you indulge me as to white it is necessary in this case to use loops for the control structures?

[gooseantlers]

Subsonics. I removed that break statement and just had it execute the XOR and no changes. I'm truly stumped.

[oogabooga]

You should open the files in binary mode ("rb", "wb").

You should check to make sure the files actually opened.

This should be sizeof(char) instead of sizeof(string)

	char*string = (char*)malloc(size*sizeof(string));

You don't want the semicolon at the end of this line

			if (fgets(userchoice, sizeof userchoice, stdin) != NULL);

(although what if it is NULL?)

You want this to be x < size

				for(int x=0; x <= size; x++)

And most important of all, under the decrpyt algorithm you have

				firstpass_encrypt(key, string, size);

but you want

				firstpass_encrypt(string, key, size);

(same as for encrypt).

[gooseantlers]

oogbooga. Thanks man. That got me on the right track. I will change my control structures over to loops instead of gotos and create more functions to get better more readable code. It messes up a little bit now. I think I'm on the right track to fix it. Thanks a bunch again

[oogabooga]

No problem. But take note of quzah's post.
E.g., try encrypting and decrypting:

ABCDEFGHIJKLMNOPQRSTUVWXYZ

[Subsonics]

That should work as long as he doesn't break for \0.

[oogabooga]

But he is breaking for 0!
So he should change his code to this:

void firstpass_encrypt(char string[], char key[],int size)
{
	for(position=0; position < size; position++)
		{
			string[position]=string[position]^key[position%62];
		}
}

Also, position shouldn't be global.