4
\$\begingroup\$

I'm attempting to transliterate a piece of encryption/decryption code written in python into C#; the code works on save data from a particular game. I can't code in python at all, but by referring to the python documentation I was able to code up a working implementation in C#. What I'd like to know is:

  1. Does the C# code properly do the functional equivalent? (I have tested the code and it gives the correct binary output, at least for the inputs I've tested)
  2. Can the C# code be improved? (All aspects welcome: coding style, algorithm...etc)

Note1: I used an external library for the BlowFish implementation, and I've included the relevant method signatures

Note2: Input and output byte arrays can be assumed to always be little endian

Original Python

class SaveDataEncryptDecrypt:
    def __init__(self):
        self._cipher = Blowfish.new(b'my key')

    def _xor(self, buff, key):
        buff = array.array('H', buff)
        for i in range(len(buff)):
            if key == 0:
                key = 1
            key = key * 0xb0 % 0xff53
            buff[i] ^= key
        return buff.tostring()

    def encrypt(self, buff):
        csum = sum(bytearray(buff)) & 0xffffffff
        buff = array.array('I', buff)
        buff.insert(0, csum)
        seed = random.getrandbits(16)
        buff = array.array('I', self._xor(buff.tostring(), seed))
        buff.insert(0, (seed << 16) + 0x10)
        buff.byteswap()
        buff = array.array('I', self._cipher.encrypt(buff.tostring()))
        buff.byteswap()
        buff = buff.tostring()
        return buff

    def decrypt(self, buff):
        buff = array.array('I', buff)
        buff.byteswap()
        buff = array.array('I', self._cipher.decrypt(buff.tostring()))
        buff.byteswap()
        seed = buff.pop(0) >> 16
        buff = array.array('I', self._xor(buff.tostring(), seed))
        csum = buff.pop(0)
        buff = buff.tostring()
        if csum != (sum(bytearray(buff)) & 0xffffffff):
            raise ValueError('Invalid checksum in header.')
        return buff

C#

public class Blowfish
{
    //class from outside library, with method signatures:
    public Blowfish(byte[] key);
    public void Decipher(byte[] buffer, int length);
    public void Encipher(byte[] buffer, int length);
}

public static class SaveDataEncryptDecrypt
{
    private static readonly Blowfish _BlowFish = new Blowfish(Encoding.ASCII.GetBytes("my key"));

    public static byte[] Decrypt(byte[] buffer)
    {
        //make sure we don't modify buffer from caller
        buffer = buffer.Clone() as byte[];

        //blowfish decrypt in big endian
        ByteSwapUIntBuffer(buffer);
        _BlowFish.Decipher(buffer, buffer.Length);
        ByteSwapUIntBuffer(buffer);

        //pop 4 byte seed and XOR
        uint seed = BitConverter.ToUInt32(buffer, 0) >> 16;
        buffer = buffer.Skip(4).ToArray();
        XOR(buffer, seed);

        //pop 4 byte checksum and make sure decrypt was successful
        uint checksum = BitConverter.ToUInt32(buffer, 0);
        buffer = buffer.Skip(4).ToArray();
        if (checksum != GetChecksum(buffer))
            throw new ArgumentException("CHECKSUM_MISMATCH");

        return buffer;
    }

    public static byte[] Encrypt(byte[] buffer)
    {
        //calculate checksum and prepend
        uint checksum = GetChecksum(buffer);
        buffer = BitConverter.GetBytes(checksum).Concat(buffer).ToArray();

        //generate a 16bit seed and XOR
        byte[] seedBuffer = new byte[2];
        new RNGCryptoServiceProvider().GetBytes(seedBuffer);
        uint seed = BitConverter.ToUInt16(seedBuffer, 0);
        XOR(buffer, seed);

        //prepend seed
        seed = (seed << 16) + 0x10;
        buffer = BitConverter.GetBytes(seed).Concat(buffer).ToArray();

        //blowfish encrypt in big endian
        ByteSwapUIntBuffer(buffer);
        _BlowFish.Encipher(buffer, buffer.Length);
        ByteSwapUIntBuffer(buffer);

        return buffer;
    }

    private static void ByteSwapUIntBuffer(byte[] buffer)
    {
        for (int i = 0; i < buffer.Length; i += 4)
        {
            Array.Reverse(buffer, i, 4);
        }
    }

    private static void XOR(byte[] buffer, uint seed)
    {
        for (int i = 0; i < buffer.Length; i += 2)
        {
            ushort segment = BitConverter.ToUInt16(buffer, i);

            if (seed == 0)
            {
                seed = 1;
            }

            seed = seed * 0xB0 % 0xFF53;
            segment ^= unchecked((ushort)seed);

            byte[] segmentBuffer = BitConverter.GetBytes(segment);
            buffer[i] = segmentBuffer[0];
            buffer[i + 1] = segmentBuffer[1];
        }
    }

    private static uint GetChecksum(byte[] buffer)
    {
        unchecked
        {
            uint sum = 0;

            for (int i = 0; i < buffer.Length; i++)
            {
                sum += buffer[i];
            }

            return sum;
        }
    }
}
\$\endgroup\$
0

1 Answer 1

1
\$\begingroup\$
  1. I would not make SaveDataEncryptDecrypt a static class. There is no real value in doing so and it has a good chance of long term pain. Static classes create implicit dependencies which have the tendency to nip you in the butt sooner or later.

    As such you should make it non-static and pass the encryption key as a constructor parameter. Now you can actually use it with different keys without having to change the code all the time.

    You could argue YAGNI but static classes pretty much always violate the D part of SOLID and usually end up violating the O part as well as a consequence of how they are being used. Following SOLID engineering principles usually ends up creating more robust and maintainable code.

  2. I usually avoid modifying the method arguments. It can make debugging annoying. You won't lose anything by storing the array copy in a local variable instead.

\$\endgroup\$
1
  • \$\begingroup\$ Good points all around. I made the class static because it pretty much only supports that one game; making it compatible with anything else will require modifying most of the functions. Still, your point stands, and I now think making it non-static is probably a better idea. \$\endgroup\$
    – Setsu
    Feb 8, 2016 at 17: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.