Help with my XOR routine..

This is a discussion on Help with my XOR routine.. within the C++ Programming forums, part of the General Programming Boards category; I know that this topic has been covered a million times already, but i went through all the XOR topics ...

  1. #1
    Internet Superhero
    Join Date
    Sep 2006
    Location
    Denmark
    Posts
    964

    Help with my XOR routine..

    I know that this topic has been covered a million times already, but i went through all the XOR topics on the site without figuring out what i am doing wrong in my own program?

    I've attached the main program file to my post, the XOR function is defined in it's own file:

    Code:
    /* Source file for the XOR routine */
    
    /* Include the function prototype */
    #include "xor.h"
    
    /* XOR-funtion declaration */
    std::string XOR(std::string line, std::string key)
    {
    		unsigned int i = 0, j = 0;					
    		std::size_t l_length, k_length;				
    		std::string e_line;		
    					
    	l_length = line.length();
    	k_length = key.length();
    	
    	for(;i < l_length; i++, j++)					
    	{
    		e_line[i] = line[i] ^ key[j];				
    		
    		if(j == k_length)							
    		{
    			j = 0;
    		}
    	}
    
    	/* Return the encrypted line */
    	return e_line;
    }
    Removed most of my comments since the forums are screwing up indentation.


    I compile my project from the command line like this:
    "g++ -omain.exe -Wall -pedantic xor.cpp main.cpp"

    But when i run the program, nothing happens after i enter the key, i don't get any output to the outputfile, and the program just closes. Where in my code is the problem?

    EDIT: Accidentally wrote "crashes" rather than "closes", my program doesn't crash or segfault or anything, it just closes without outputting to the outputfile.
    Attached Files Attached Files
    Last edited by Neo1; 10-04-2007 at 06:11 AM.

  2. #2
    Registered User hk_mp5kpdw's Avatar
    Join Date
    Jan 2002
    Location
    Northern Virginia/Washington DC Metropolitan Area
    Posts
    3,793
    What does your input file look like? What is the key you are attempting to use?

    Code:
    std::string inputfilepath, outputfilepath;		/* Used for holding the file paths */
    
    ...
    	
    /* Assign the input from the cmd-line to the filepath strings */
    inputfilepath = argv[1];
    outputfilepath = argv[2];
    	
    /* Open the stream to the input file */
    std::ifstream inputfile(inputfilepath.c_str(), std::ios::binary);
    	
    ...
    	
    /* If we got this far, then input file is valid, so we can now start to write to the outputfile */
    std::ofstream outputfile(outputfilepath.c_str(), std::ios::binary);
    Not that it's the issue here, but there appears to be no real reason for the inputfilepath and outputfilepath string variables. You can get rid of them and just use argv[1] and argv[2] directly:
    Code:
    /* Open the stream to the input file */
    std::ifstream inputfile(argv[1], std::ios::binary);
    	
    ...
    	
    /* If we got this far, then input file is valid, so we can now start to write to the outputfile */
    std::ofstream outputfile(argv[2], std::ios::binary);
    Last edited by hk_mp5kpdw; 10-04-2007 at 05:53 AM.
    "Owners of dogs will have noticed that, if you provide them with food and water and shelter and affection, they will think you are god. Whereas owners of cats are compelled to realize that, if you provide them with food and water and shelter and affection, they draw the conclusion that they are gods."
    -Christopher Hitchens

  3. #3
    Internet Superhero
    Join Date
    Sep 2006
    Location
    Denmark
    Posts
    964
    Quote Originally Posted by hk_mp5kpdw View Post
    What does your input file look like? What is the key you are attempting to use?

    Code:
    std::string inputfilepath, outputfilepath;		/* Used for holding the file paths */
    
    ...
    	
    /* Assign the input from the cmd-line to the filepath strings */
    inputfilepath = argv[1];
    outputfilepath = argv[2];
    	
    /* Open the stream to the input file */
    std::ifstream inputfile(inputfilepath.c_str(), std::ios::binary);
    	
    ...
    	
    /* If we got this far, then input file is valid, so we can now start to write to the outputfile */
    std::ofstream outputfile(outputfilepath.c_str(), std::ios::binary);
    Not that it's the issue here, but there appears to be no real reason for the inputfilepath and outputfilepath string variables. You can get rid of them and just use argv[1] and argv[2] directly:
    Code:
    /* Open the stream to the input file */
    std::ifstream inputfile(argv[1], std::ios::binary);
    	
    ...
    	
    /* If we got this far, then input file is valid, so we can now start to write to the outputfile */
    std::ofstream outputfile(argv[2], std::ios::binary);
    The files are called foo.txt and bar.txt.

    Foo contains the string: "This doesnt work!", and bar is empty, the key im attempting to use is simply "neo1", i've tried alot of other combinations but to no prevail.

    I am using the strings for the file names because it makes the code more descriptive imo, i won't need to explain everything to whoever reads the code, if the variable names do it for me. And besides, speed/size is not important at all in this small application.

  4. #4
    Registered User hk_mp5kpdw's Avatar
    Join Date
    Jan 2002
    Location
    Northern Virginia/Washington DC Metropolitan Area
    Posts
    3,793
    Code:
    std::string XOR(std::string line, std::string key)
    {
        unsigned int i = 0, j = 0;					
        std::size_t l_length, k_length;				
        std::string e_line;		
    					
        l_length = line.length();
        k_length = key.length();
    	
        for(;i < l_length; i++, j++)					
        {
            e_line[i] = line[i] ^ key[j];				
    		
            if(j == k_length)							
            {
                j = 0;
            }
        }
    
        /* Return the encrypted line */
        return e_line;
    }
    I put all your code into a test project and ran it in the debugger (Visual Studio 2005). The e_line[i] = line[i] ^ key[j] line is where the code stopped with an error and it's now obvious what the issue is. Do you see?
    "Owners of dogs will have noticed that, if you provide them with food and water and shelter and affection, they will think you are god. Whereas owners of cats are compelled to realize that, if you provide them with food and water and shelter and affection, they draw the conclusion that they are gods."
    -Christopher Hitchens

  5. #5
    Internet Superhero
    Join Date
    Sep 2006
    Location
    Denmark
    Posts
    964
    Quote Originally Posted by hk_mp5kpdw View Post
    Code:
    std::string XOR(std::string line, std::string key)
    {
        unsigned int i = 0, j = 0;					
        std::size_t l_length, k_length;				
        std::string e_line;		
    					
        l_length = line.length();
        k_length = key.length();
    	
        for(;i < l_length; i++, j++)					
        {
            e_line[i] = line[i] ^ key[j];				
    		
            if(j == k_length)							
            {
                j = 0;
            }
        }
    
        /* Return the encrypted line */
        return e_line;
    }
    I put all your code into a test project and ran it in the debugger (Visual Studio 2005). The e_line[i] = line[i] ^ key[j] line is where the code stopped with an error and it's now obvious what the issue is. Do you see?
    Yea, the e_line string is empty, so it crashes when i use the [] operator on the string, right?

    What can i do to fix this? Should i fill the string with '0's or something? Can i use the += operator?

  6. #6
    Registered User hk_mp5kpdw's Avatar
    Join Date
    Jan 2002
    Location
    Northern Virginia/Washington DC Metropolitan Area
    Posts
    3,793
    Yeah, a couple, one:
    Code:
    e_line.push_back(line[i] ^ key[j]);
    Or maybe use resize on the string or append or when first declaring the string you can set it's size and fill it with spaces to start off? += would probably work as well.
    "Owners of dogs will have noticed that, if you provide them with food and water and shelter and affection, they will think you are god. Whereas owners of cats are compelled to realize that, if you provide them with food and water and shelter and affection, they draw the conclusion that they are gods."
    -Christopher Hitchens

  7. #7
    Internet Superhero
    Join Date
    Sep 2006
    Location
    Denmark
    Posts
    964
    Quote Originally Posted by hk_mp5kpdw View Post
    Yeah, a couple, one:
    Code:
    e_line.push_back(line[i] ^ key[j]);
    Or maybe use resize on the string or append or when first declaring the string you can set it's size and fill it with spaces to start off? += would probably work as well.
    Awesome, i'm now getting stuff written to the file

    Still not quite done yet though, when i try to encrypt and then decrypt "this doesnt work!", the output i get is "This d~yHT"~yH (- And more stuff that that the forums can't show without bugging). Something is still wrong with the XOR function?

  8. #8
    Registered User hk_mp5kpdw's Avatar
    Join Date
    Jan 2002
    Location
    Northern Virginia/Washington DC Metropolitan Area
    Posts
    3,793
    First, you had a subtle problem:
    Code:
    std::string XOR(std::string line, std::string key)
    {
        unsigned int i = 0, j = 0;					
        std::size_t l_length, k_length;				
        std::string e_line;		
    					
        l_length = line.length();
        k_length = key.length();
    	
        for(;i < l_length; i++, j++)					
        {
            e_line.push_back(line[i] ^ key[j]);
    		
            if(j == k_length)							
            {
                j = 0;
            }
        }
    
        /* Return the encrypted line */
        return e_line;
    }
    The problem is you increment j and use it with the key as an index to encrypt the string, then you check and make sure it's within bounds. The bounds check should be before you use it, so try this instead:
    Code:
    std::string XOR(std::string line, std::string key)
    {
        unsigned int i = 0, j = 0;					
        std::size_t l_length, k_length;				
        std::string e_line;		
    					
        l_length = line.length();
        k_length = key.length();
    	
        for(;i < l_length; i++, j++)					
        {
            if(j == k_length)							
            {
                j = 0;
            }
    
            e_line.push_back(line[i] ^ key[j]);
        }
    
        /* Return the encrypted line */
        return e_line;
    }

    Now the main event. That's a problem with the way you are doing the encryption/decryption using getline. getline looks for a trailing newline character (or end of file) and then stops. When you first encrypt your string, the getline successfully reads the whole string and the 'o' in "work!" happens to get encrypted as a ASCII value 10 which is either a carriage return or line feed (I forget which). When it comes time to decrypt the string, getline stops at that character prematurely (extracts and throws away). Because of this, a second getline call is needed to read and decrypt the rest of the file. Since your encrypt/decrypt method works on whole lines, this second round of decryption starts using the wrong key index and the wrong cypher text character from what was used in the encryption step. Bottom line here is I don't think you should be using getline when doing something like this. Getting the characters one-by-one or getting all the characters at once (using an intermediate stringstream object and rdbuf) and then encrypting them with the key might be best here.

    I'd do something like this for the reading instead of the while loop and getline calls:
    Code:
    #include <sstream>
    
    ...
    
    std::stringstream sstr;
    sstr << inputfile.rdbuf();  // Read whole file in one go, no loop needed
    line = sstr.str();  // Convert stringstream to string
    	
    line = XOR(line, key);
    
    outputfile << line;
    Last edited by hk_mp5kpdw; 10-04-2007 at 01:54 PM.
    "Owners of dogs will have noticed that, if you provide them with food and water and shelter and affection, they will think you are god. Whereas owners of cats are compelled to realize that, if you provide them with food and water and shelter and affection, they draw the conclusion that they are gods."
    -Christopher Hitchens

  9. #9
    Internet Superhero
    Join Date
    Sep 2006
    Location
    Denmark
    Posts
    964
    Quote Originally Posted by hk_mp5kpdw View Post
    First, you had a subtle problem:
    Code:
    std::string XOR(std::string line, std::string key)
    {
        unsigned int i = 0, j = 0;					
        std::size_t l_length, k_length;				
        std::string e_line;		
    					
        l_length = line.length();
        k_length = key.length();
    	
        for(;i < l_length; i++, j++)					
        {
            e_line.push_back(line[i] ^ key[j]);
    		
            if(j == k_length)							
            {
                j = 0;
            }
        }
    
        /* Return the encrypted line */
        return e_line;
    }
    The problem is you increment j and use it with the key as an index to encrypt the string, then you check and make sure it's within bounds. The bounds check should be before you use it, so try this instead:
    Code:
    std::string XOR(std::string line, std::string key)
    {
        unsigned int i = 0, j = 0;					
        std::size_t l_length, k_length;				
        std::string e_line;		
    					
        l_length = line.length();
        k_length = key.length();
    	
        for(;i < l_length; i++, j++)					
        {
            if(j == k_length)							
            {
                j = 0;
            }
    
            e_line.push_back(line[i] ^ key[j]);
        }
    
        /* Return the encrypted line */
        return e_line;
    }

    Now the main event. That's a problem with the way you are doing the encryption/decryption using getline. getline looks for a trailing newline character (or end of file) and then stops. When you first encrypt your string, the getline successfully reads the whole string and the 'o' in "work!" happens to get encrypted as a ASCII value 10 which is either a carriage return or line feed (I forget which). When it comes time to decrypt the string, getline stops at that character prematurely (extracts and throws away). Because of this, a second getline call is needed to read and decrypt the rest of the file. Since your encrypt/decrypt method works on whole lines, this second round of decryption starts using the wrong key index and the wrong cypher text character from what was used in the encryption step. Bottom line here is I don't think you should be using getline when doing something like this. Getting the characters one-by-one or getting all the characters at once (using an intermediate stringstream object and rdbuf) and then encrypting them with the key might be best here.

    I'd do something like this for the reading instead of the while loop and getline calls:
    Code:
    #include <sstream>
    
    ...
    
    std::stringstream sstr;
    sstr << inputfile.rdbuf();  // Read whole file in one go, no loop needed
    line = sstr.str();  // Convert stringstream to string
    	
    line = XOR(line, key);
    
    outputfile << line;
    Thanks alot, it's working perfectly now !

    Would you mind explaining what exactly "rdbuf()" does? Also, is there anything else in my code that could be improved?

  10. #10
    Registered User mikeman118's Avatar
    Join Date
    Aug 2007
    Posts
    183
    Alternatively you could do the following:
    Code:
    string LoadText(string fileName)
    {
    	int length;
    	string outputString;
    	char * outputChar;
    	
    	ifstream is;
    	is.open (fileName.c_str(), ios::binary );
    
    	// get length of file:
    	is.seekg (0, ios::end);
    	length = is.tellg();
    	is.seekg (0, ios::beg);
    
      // allocate memory:
            outputChar= new char [length];
    
      // read data as a block:
    	is.read (outputChar,length);
    	is.close();
    	outputString.assign(outputChar, length);
    	return outputString;
    }
    This is to read, the next code is to write:
    Code:
    void SaveText(string fileName, string text)
    {
    	ofstream out(fileName.c_str(), ios::binary);
    	int i = text.size();
    	out.write(text.c_str(), i);
    	out.close();
    }
    For both, fileName is obviously the file to read/write, and in the SaveText the string text is the info to write.
    I assume that they do the same thing (hk's example), but I thought this might be useful as this is what I use for Xor encryption, to encrypt/decrypt files other than text files (such as a video or something). Hope this helps.

Popular pages Recent additions subscribe to a feed

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21