XOR encryption and string conversion

[Layosh]

I implemented the simplest form of XOR encryption. The XOR operation with the clrea text and the key resulst in non-ascii characters, that is an issue when sending the encrypted text. I chose to convert the unsigned char string into a HEXADECIMAL character string, only 0-9 A-F characters there and they play nice. For concersion I used the sprinf function, and the output seems to be correct.

However when decrypt the HEXADECIMAL string, I need to convert the two byte Hexadecimal "characters back into unsigned char values, and that is when I fail to find a solution, the sscanf function keep generating General protection failure at the very first instance of the conversion (I, j=0).  I tried various formatting parameters "0x", "02X", "02hhx", all fail. I am out of ideas.

I looked up the formatting string and either I miss something or the datatype are wrong but it compiles with no error.

/*XOR encrypts message with the key (an MD5 hash) and returns the pointer to the allocated encrypted hexadecimal string*/
unsigned char *  XorEncode (const  char * message, char * key) {
int i=0,j=0;  
size_t messagelen=0;
unsigned char *hexstringbuffer;
messagelen = strlen(message);
 

if(messagelen<=1|| strlen(key)<1) return NULL;//Error in parameters
hexstringbuffer=malloc(((messagelen*2)+1));
 memset(hexstringbuffer,'\0',2*messagelen+2);

 //convert the encrypted buffer to hexadecimal string
 for(i = 0,j=0; i < messagelen; i++,j+=2) {
      sprintf(&hexstringbuffer[j],"%02x", message[i] ^ key[i % (sizeof(key)/sizeof(char))]);// works well with or without sprinf conversion 
 }
 hexstringbuffer[j-2]='\0';  //for j incremented before exiing the loop
 return hexstringbuffer; //user must free the encrypted buffer, do not want to store it when out of scope
}
//decryptes the message of a hex stringn eeds the same key as the excription

//need to convert back the hexadecimal (ascii) array to unsigned char array

// fgor this reason I need separate decode function;

//all works if the hexadecimal conversion is omitted

unsigned char * XorDecode(const  char * encryptedmessage, char * key) {
int i,j;
 size_t messagelen=0;
 unsigned int bytebuffer='0';
 messagelen=strlen(encryptedmessage);
 char *decrypted=malloc (sizeof(char)*(messagelen/2)+1); 
 
  for(i=0,j=0; i < messagelen; i++,j+=2) {
         sscanf(encryptedmessage[j],"%02x",&bytebuffer);    // I GET A GENERAL PROTECTION FAULT HERE
         decrypted[i]=bytebuffer ^ key[i % (sizeof(key) / sizeof(char))];
  }
 decrypted[i]='\0';
 return decrypted; //the user must free the decrypted buffer
}

[mkossmann]

Your upper limit in the for loop is to high. If i == messagelen, j is 2*messagelen. But your encrypted string contains only messagelen chars

Either use i<messagelen/2 or j < messagelen

[RobertoBozzolo]

Some additional errors are present in your code:

1. You should receive a compilation warning in the line

sscanf (encryptedmessage[j], "%02x", &ch);

warning: incompatible integer to pointer conversion passing 'const char' to parameter of type 'const char *'; take the address with &

2. The line

hexstringbuffer[j-2]='\0';  //for j incremented before exiting the loop

is useless since you already filled the string with zeroes; it is moreover harmful since it rewrites a legitimate character in the output string (you should have noted that the output string is 2 bytes shorter than expected)

Having corrected these and other minor bugs, the resulting code works as expected:

unsigned char *  XorEncode (const  char * message, char * key)
{
	int		i, j;   
	size_t	messagelen = strlen(message);
	unsigned char *hexstringbuffer;

	if (messagelen <= 1 || strlen (key) < 1) return NULL;//Error in parameters

	hexstringbuffer = malloc (messagelen * 2 + 2); 
	memset (hexstringbuffer, '\0', 2 * messagelen + 2);

	//convert the encrypted buffer to hexadecimal string
	for (i = 0, j = 0; i < messagelen; i++, j += 2) {
		DebugPrintf ("%d: %c - ", i, message[i]);
		sprintf (&hexstringbuffer[j], "%02x", message[i] ^ key[i % (sizeof(key) / sizeof(char))]);
		DebugPrintf ("%2s\n", hexstringbuffer + j);
	}
//	hexstringbuffer[j-2]='\0';  // incorrect!

	return hexstringbuffer;
}

unsigned char * XorDecode (const  char * encryptedmessage, char * key)
{
	int		i, j, ch;
	size_t	messagelen = strlen(encryptedmessage);
	unsigned char	bytebuffer = '0';

	char *decrypted = malloc (sizeof (char) * (messagelen / 2) + 2); 
 
	for (i = 0, j = 0; j < messagelen; i++, j += 2) {
//		sscanf (encryptedmessage[j], "%02x", &bytebuffer);    // I GET A GENERAL PROTECTION FAULT HERE
//		decrypted[i] = bytebuffer ^ key[i % (sizeof(key) / sizeof(char))]; 
		sscanf  (encryptedmessage[j], "%02x", &ch);
		DebugPrintf ("%d: %02x - ", j, ch);
		ch ^= key[i % (sizeof(key) / sizeof(char))];
		DebugPrintf ("%c\n", ch);
		sprintf (&decrypted[i], "%c", ch);; 
	}
	decrypted[i] = '\0';

	return decrypted; //the user must free the decrypted buffer
}

[Layosh]

Thanks for the help. I compiled the corrected code and got the warning as expected. However the line:

       sscanf (encryptedmessage[j], "%02x", &ch);

still generated General Protection Fault in the very first iteration where i and j are still 0. 

If  int ch is changed from int to "unsigned char ch", no GPE but a parameter mismatch warning pops up, getting char pointer instead of int pointer. The sscanf conversion results in /0, and XOR converts it to '2' instead of 'A'.

I am still at loss...

[Layosh]

Actually, my encrypted sting is in hexadecimal format (two of 0-9 or a-f characters) and therefore two characters correspond to one char byte in the message, and the entire length of the encoded string representation is 2 times the message length.  

My problem is with the sscanf function in the XORDecrypt (). It is still croaking on me...

[RobertoBozzolo · Accepted solution]

Sorry, I posted a wrong line of code. The correct syntax is

sscanf  (&encryptedmessage[j], "%02x", &ch);

as can be derived from the warning message I posted earlier ("take the address with &")

[Layosh]

This ampersand is what the doctor ordered!

Thank you!