4
\$\begingroup\$

I decided to roll out my own encryption algorithm, which I am calling "SBC" or Simple Byte Cipher. Essentially, I just take the bytes of the string data, do a Caesar cipher like shift to it, increment through an array of password bytes for each byte in the data (this enables the same letters such as "T" to not always have the same output, which would be easily crackable). And in the end I just reverse the process to glean the original data.

My code seems to work just fine; decryption works like a dream. I just want to know if there is a better way, or any hidden errors that may occur with the code as it currently stands.

I am aware that the length of the password and data are not length tested, or null tested. I will address that later.

public static class SimpleByteCipher
    {
        public static byte[] EncryptStringToByteArray( string data , string password )
        {
            byte[] bytes = Encoding.ASCII.GetBytes( data );
            byte[] passwordBytes = Encoding.ASCII.GetBytes( password );
            int passwordShiftIndex = 0;
            for( int i = 0; i < bytes.Length; i++ )
            {
                bytes[ i ] = ( byte )( bytes[ i ] + passwordBytes[ passwordShiftIndex ] );
                passwordShiftIndex = ( passwordShiftIndex + 1 ) % passwordBytes.Length;
            }
            return bytes;
        }

        public static string DecryptByteArrayToString( byte[] data , string password )
        {
            byte[] bytes = data;
            byte[] passwordBytes = Encoding.ASCII.GetBytes( password );
            int passwordShiftIndex = 0;
            for( int i = 0; i < bytes.Length; i++ )
            {
                bytes[ i ] = ( byte )( bytes[ i ] - passwordBytes[ passwordShiftIndex ] );
                passwordShiftIndex = ( passwordShiftIndex + 1 ) % passwordBytes.Length;
            }
            return Encoding.ASCII.GetString( bytes );
        }

        public static byte[] EncryptStringToByteArray( string data , string password , uint seed)
        {
            byte[] bytes = Encoding.ASCII.GetBytes( data );
            byte[] passwordBytes = Encoding.ASCII.GetBytes( password );
            int passwordShiftIndex = 0;
            for( int i = 0; i < bytes.Length; i++ )
            {
                bytes[ i ] = ( byte )( bytes[ i ] + passwordBytes[ passwordShiftIndex ] + seed );
                passwordShiftIndex = ( passwordShiftIndex + 1 ) % passwordBytes.Length;
            }
            return bytes;
        }

        public static string DecryptByteArrayToString( byte[] data , string password , uint seed)
        {
            byte[] bytes = data;
            byte[] passwordBytes = Encoding.ASCII.GetBytes( password );
            int passwordShiftIndex = 0;
            for( int i = 0; i < bytes.Length; i++ )
            {
                bytes[ i ] = ( byte )( bytes[ i ] - passwordBytes[ passwordShiftIndex ] - seed );
                passwordShiftIndex = ( passwordShiftIndex + 1 ) % passwordBytes.Length;
            }
            return Encoding.ASCII.GetString( bytes );
        }
    }

And here is a practice main:

class Program
    {
        static void Main( string[] args )
        {
            string data = "This is but a test.";
            byte[] encrypted = SimpleByteCipher.EncryptStringToByteArray( data , "Test" , 5000 );
            string decrypted = SimpleByteCipher.DecryptByteArrayToString( encrypted , "Test" , 5000 );
            Console.WriteLine( decrypted );
            Console.ReadLine();
        }
     }

The output correctly prints:

"This is but a test."

And here is a visualization of what the encrypted bytes look like in a text file:enter image description here

\$\endgroup\$
  • \$\begingroup\$ I don't get to say this often, but you have a great use of horizontal whitespace. Kudos. Could use a bit more vertical space though. Use it to group together small bits of code that logically group together, but shouldn't be broken into their own methods (for whatever reason). \$\endgroup\$ – RubberDuck Jul 29 '15 at 1:33
  • 1
    \$\begingroup\$ @RubberDuck I am extremely anal about how my code looks. Call it OCD, but it seriously bothers me when things are not appropriately separated by spaces. \$\endgroup\$ – Krythic Jul 29 '15 at 1:36
  • \$\begingroup\$ At the risk of getting chatty, is there a way to configure VS to add the extra space around indexes like you've done here? I really like that. \$\endgroup\$ – RubberDuck Jul 29 '15 at 1:37
  • 1
    \$\begingroup\$ @RubberDuck Yes. I have it set up so I just hit Ctrl+K+D and it does it all for me in split second. \$\endgroup\$ – Krythic Jul 29 '15 at 1:44
  • 1
    \$\begingroup\$ It should also be noted for anyone who chooses to use this code, that when you print it to a document with WriteAllBytes, it has the gibberish appearance of binary files. =D \$\endgroup\$ – Krythic Jul 29 '15 at 2:04
5
\$\begingroup\$

Reduce Duplication

There appears to be some significant code duplication between the encrypt and decrypt methods.

Method Overloads

The first and most obvious spots we can refactor are the overloads for your decrypt and encrypt functions:

public static byte[] EncryptStringToByteArray( string data , string password )
{
    byte[] bytes = Encoding.ASCII.GetBytes( data );
    byte[] passwordBytes = Encoding.ASCII.GetBytes( password );
    int passwordShiftIndex = 0;
    for( int i = 0; i < bytes.Length; i++ )
    {
        bytes[ i ] = ( byte )( bytes[ i ] + passwordBytes[ passwordShiftIndex ] );
        passwordShiftIndex = ( passwordShiftIndex + 1 ) % passwordBytes.Length;
    }
    return bytes;
}

public static byte[] EncryptStringToByteArray( string data , string password , uint seed)
{
    byte[] bytes = Encoding.ASCII.GetBytes( data );
    byte[] passwordBytes = Encoding.ASCII.GetBytes( password );
    int passwordShiftIndex = 0;
    for( int i = 0; i < bytes.Length; i++ )
    {
        bytes[ i ] = ( byte )( bytes[ i ] + passwordBytes[ passwordShiftIndex ] + seed );
        passwordShiftIndex = ( passwordShiftIndex + 1 ) % passwordBytes.Length;
    }
    return bytes;
}

The only difference between these two is that one takes in a seed and adds it to the bytes within the loop. The overload without a seed can be rewritten as follows:

public static byte[] EncryptStringToByteArray(string data, string password)
{
    return EncryptStringToByteArray(data, password, 0);
}

A similar refactor can be done with the two decrypt overloads. I will leave that step as an exercise for the reader.

Symmetric Encryption

With what's left, I cannot help but notice that this appears to be a symmetric encryption algorithm. As a result, the encrypt and decrypt steps are nearly identical to one another:

public static byte[] EncryptStringToByteArray( string data , string password , uint seed)
{
    byte[] bytes = Encoding.ASCII.GetBytes( data );
    byte[] passwordBytes = Encoding.ASCII.GetBytes( password );
    int passwordShiftIndex = 0;
    for( int i = 0; i < bytes.Length; i++ )
    {
        bytes[ i ] = ( byte )( bytes[ i ] + passwordBytes[ passwordShiftIndex ] + seed );
        passwordShiftIndex = ( passwordShiftIndex + 1 ) % passwordBytes.Length;
    }
    return bytes;
}

public static string DecryptByteArrayToString( byte[] data , string password , uint seed)
{
    byte[] bytes = data;
    byte[] passwordBytes = Encoding.ASCII.GetBytes( password );
    int passwordShiftIndex = 0;
    for( int i = 0; i < bytes.Length; i++ )
    {
        bytes[ i ] = ( byte )( bytes[ i ] - passwordBytes[ passwordShiftIndex ] - seed );
        passwordShiftIndex = ( passwordShiftIndex + 1 ) % passwordBytes.Length;
    }
    return Encoding.ASCII.GetString( bytes );
}

They do, however, have some subtle differences:

  • Encrypt adds the seed, while Decrypt subtracts it
  • Encrypt takes in string and returns byte[], while Decrypt takes in byte[] and returns string.

However, neither are particularly large differences. We can refactor further if we extract the following method:

private static byte[] Encrypt(byte[] bytes, string password, uint seed, int factor)
{
    var passwordBytes = Encoding.ASCII.GetBytes(password);
    var passwordShiftIndex = 0;
    for(int i = 0; i < bytes.Length; i++)
    {
        bytes[i] = (byte) (bytes[i] + factor * (passwordBytes[passwordShiftIndex] + seed));
        passwordShiftIndex = (passwordShiftIndex + 1) % passwordBytes.Length;
    }
    return bytes;
}

Note the following changes:

  • It takes in and returns byte[]
  • I added an integer factor variable which is used to control whether passwordBytes[passwordShiftIndex] and seed are added or subtracted

With those changes, we can refactor our seeded encrypt and decrypt methods to call the above method, changing the class as follows:

public static class SimpleByteCipher
{
    public static byte[] EncryptStringToByteArray(string data, string password)
    {
        return EncryptStringToByteArray(data, password, 0);
    }

    public static byte[] EncryptStringToByteArray(string data, string password, uint seed)
    {
        var bytes = Encoding.ASCII.GetBytes(data);
        return Encrypt(bytes, password, seed, 1);
    }

    private static byte[] Encrypt(byte[] bytes, string password, uint seed, int factor)
    {
        var passwordBytes = Encoding.ASCII.GetBytes(password);
        var passwordShiftIndex = 0;
        for(int i = 0; i < bytes.Length; i++)
        {
            bytes[i] = (byte) (bytes[i] + factor * (passwordBytes[passwordShiftIndex] + seed));
            passwordShiftIndex = (passwordShiftIndex + 1) % passwordBytes.Length;
        }
        return bytes;
    }

    public static string DecryptByteArrayToString(byte[] data, string password)
    {
        return DecryptByteArrayToString(data, password, 0);
    }

    public static string DecryptByteArrayToString(byte[] data, string password, uint seed)
    {
        var bytes = Encrypt(data, password, seed, -1);
        return Encoding.ASCII.GetString(bytes);
    }
}
\$\endgroup\$
  • \$\begingroup\$ I like your suggestions, except for the overzealous use of the var keyword. \$\endgroup\$ – Krythic Jul 29 '15 at 18:02
  • 1
    \$\begingroup\$ In your (de|en)crypt simplifiction you missed that passwordBytes[ passwordShiftIndex ] in also flipped from added to subtracted. \$\endgroup\$ – Shaun H Jul 29 '15 at 18:07
  • \$\begingroup\$ @ShaunH I think your comment got cut off, but I think I see what you're getting at - I missed the sign change for passwordBytes. Good catch. \$\endgroup\$ – Dan Lyons Jul 29 '15 at 18:09
  • \$\begingroup\$ @DanLyons Yeah grabbed what i wanted and started to write around it. Then my brain forgot that enter submits in comments. \$\endgroup\$ – Shaun H Jul 29 '15 at 18:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.