Custom encryption algorithm, "SBC"

Question

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:

Answer 1

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);
    }
}