Jump to content

Bug in a simple XOR program written in C

- - - - -

This topic has been archived. This means that you cannot reply to this topic.
4 replies to this topic

#1
ShadenSmith

ShadenSmith

    Newbie

  • Members
  • PipPip
  • 19 posts
I'm completely stumped by this bug. This is my first C program, it reads in two strings via fgets() and xor's them together.

main:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define MAX_SIZE 256

void strip_newline(char* to_strip);
void encrypt_data(char* encrypt, char* key);

int main()
{
	char* encrypt = malloc(MAX_SIZE);
	char* key = malloc(MAX_SIZE);	
	
	fgets(encrypt, sizeof(encrypt), stdin);
	fgets(key, sizeof(key), stdin);

	//strip newlines
	strip_newline(encrypt);
	strip_newline(key);

	//Debug
	printf("Encrypt: %s\n", encrypt);
	printf("Key: %s\n", key);	

	//XOR data and print
	encrypt_data(encrypt, key);	
	printf("Encrypted: %s\n", encrypt);

	//XOR data again (reversing the encryption) and print
	encrypt_data(encrypt, key);
	printf("Decrypted: %s\n", encrypt);	

	//Release memory
	free(key);
	free(encrypt);

        return 0;
}

The encryption algorithm:
void encrypt_data(char* encrypt, char* key)
{
	int key_count = 0; //Used to restart key if strlen(key) < strlen(encrypt)
	int i;
	for (i = 0; i < strlen(encrypt); i++) //Loop through each character of encrypt
	{
		//XOR the data
		encrypt[i] ^= key[key_count];
		//Increment key_count and start over if necessary
		key_count++;
		if(key_count == strlen(key))
			key_count = 0;
	}
}

To strip newlines:
void strip_newline(char* to_strip)
{
	//remove newlines
	if (to_strip[strlen(to_strip) - 1] == '\n')
	{
		to_strip[strlen(to_strip) - 1] = '\0';
	}
}


That's the entire xortest.c file. I'm stumped because this bug only reveals itself in specific (and seemingly unrelated) cases. For the following examples the input format is as follows:

Input1 (this is to be encrypted)
Input2 (this is the key)
Encrypt: Input1 (This is entirely for debugging purposes)
Key: Input2 (This is entirely for debugging purposes)
Encrypted: (encrypted string)
Decrypted: Input1 (after decryption)

Here are some sample calls:

./xortest
Shaden
poop
Encrypt: Shaden
Key: poop
Encrypted: #
Decrypted: Shaden
This one works great! Now for some that don't:

./xortest
Shaden
test
Encrypt: Shaden
Key: test
rypted: '

Decrypted: Shaden

It appears that the 'Encrypted' inside of my printf() function was corrupted?

Another:
./xortest
pooooooooooooooooo
Encrypt: poooooo
Key: ooooooo
Encrypted: 
Decrypted: p
Notice how I was never prompted to input the key and my first input was split in half. Also notice that no encrypted value was printed and that the decrypted value did not match the original input.

One more:
./xortest
Shaden
Smith
Encrypt: Shaden
Key: Smith
Encrypted: 
Decrypted: 

This one had no encryption output whatsoever. Does anyone have any insight as to what my bug could be?

EDIT: I found another output that seems helpful:

 ./xortest
Shaden
a
Encrypt: Shaden
Key: a
Encrypted: 2	
Decrypted: Sh

It appears that the result was terminated when the two a's were XOR'ed. This also occured in the 'poooooooo' example. When two of the same values are XOR'ed, the result is always 0. Perhaps this could be affecting the encrypted string? I'm not sure how this could relate to corrupted data directly submitted to printf() however.

Oh yeah, feel free to comment on the quality of the code (outside of the bug of course). I'm new to C, please critique!

#2
WingedPanther

WingedPanther

    A spammer's worst nightmare

  • Moderators
  • 16,831 posts
You do realize that \0 is the end of a string, right? Replacing \n with \0 will truncate your string.
Programming is a branch of mathematics.
My CodeCall Blog | My Personal Blog

#3
marwex89

marwex89

    Writes binary right handed and hex left handed

  • Members
  • PipPipPipPipPipPipPipPipPip
  • 10,720 posts
Truncating the string is fine, you'll have to do that. The problem is that you use strlen(encrypt) in your encrypt_data() function. When you XOR two identical characters, the result is 0. Guess what happens to your for-loop? strlen() says end of string is where it finds the first terminating \0.

The easiest way around this problem, is probably to do this:
void encrypt_data(char encrypt[], [B]int size[/B], char key[])
{
    int key_count = 0; 
    int i;
        
    for (i = 0; i <[B] size[/B]; i++)
    {                      
        encrypt[i] ^= key[key_count];      
        key_count++;        
        if(key_count == strlen(key))
          key_count = 0;
    }    
}

This way, when you encrypt/decrypt, you don't have to worry about the return value of strlen() changing.

You call it like this:
    /* Get size of string */
    int strsize = strlen(encrypt);
    
    /*XOR data and print*/
    encrypt_data(encrypt, strsize, key);    
    printf("Encrypted: %s\n", encrypt);
    
    /*XOR data again (reversing the encryption) and print*/
    encrypt_data(encrypt, strsize, key);
    printf("Decrypted: %s\n", encrypt);


The encrypted string will often not seem complete, because printf("%s", char*) also stops at first \0.

I kind of suck at explaining (and it's 2:37 am over here...), but just ask if there is anything.
Hey! Check out my new Toyota keyboaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

#4
dargueta

dargueta

    Writes binary right handed and hex left handed

  • Moderators
  • 4,715 posts
Also, printing out the results to the console is not a good idea. Some control characters, like backspace, will end up moving the cursor around and get you ugly things.
sudo rm -rf /

#5
ShadenSmith

ShadenSmith

    Newbie

  • Members
  • PipPip
  • 19 posts
I see what I did wrong now. Thank you all for the explanations! I'm beginning to become more comfortable with C.