Citadel's VisualEncrypt and VisualDecrypt

Question

I was skimming through this paper out of boredom. On pages 5 and 6, the paper shows a simple encryption/decryption scheme that Citadel uses to obfuscate data. The algorithm basically XORs the next character with the previous character. The code is roughly this (ugly, I know):

void VisualEncrypt (void *buffer, unsigned size) {
  for (unsigned i = 1; i < size; ++i) {
      ((unsigned char *) buffer) [i] ^= ((unsigned char *) buffer) [i - 1] ;
  }
}

void VisualDecrypt (void *buffer, unsigned size) {
  if (!size) {
      return ;
  }

  for (unsigned i = size - 1; i > 0; --i) {
      ((unsigned char *) buffer) [i] ^= ((unsigned char *) buffer) [i - 1] ;
  }
}

I decided to make a more generic version of this code so that it could work on different containers and values:

template <class InputIterator, class OutputIterator>
OutputIterator VisualEncrypt (InputIterator begin, InputIterator end, OutputIterator destination)
{
    if (begin == end) {
        return destination ;
    }

    auto value = *begin ;
    *destination++ = value ;

    while (++begin != end) {
        value ^= *begin ;
        *destination++ = value ; 
    }

    return destination ;
}

template <class InputIterator, class OutputIterator>
OutputIterator VisualDecrypt (InputIterator begin, InputIterator end, OutputIterator destination)
{
    if (begin == end) {
        return destination ;
    }

    auto value = *begin ;
    *destination++ = value ;

    while (++begin != end) {
        auto temp = *begin ;
        *destination++ = value ^ temp ;
        value = temp ;
    }

    return destination ;
}

template <class Iterator>
Iterator VisualEncrypt (Iterator begin, Iterator end)
{
    return VisualEncrypt (begin, end, begin) ;
}

template <class Iterator>
Iterator VisualDecrypt (Iterator begin, Iterator end)
{
    return VisualDecrypt (begin, end, begin) ;
}

Here are two sample cases of it being used:

#include <iterator>
#include <string>
#include <vector>

int main ()
{
    // Test char
    std::string plaintext = "Please encrypt me!" ;

    std::vector <unsigned char> cyphertext ;
    VisualEncrypt (plaintext.cbegin (), plaintext.cend (), std::back_inserter (cyphertext)) ;

    std::string decrypttext ;
    VisualDecrypt (cyphertext.cbegin (), cyphertext.cend (), std::back_inserter (decrypttext)) ;

    // Test long
    long arr [] = {1, 2, 3, 4, 5} ;
    const auto size = sizeof (arr) / sizeof (arr [0]) ;
    VisualEncrypt (arr, arr + size) ;
    VisualDecrypt (arr, arr + size) ;

    return 0 ;
}

The code seems to work fine. I want to make sure I'm not misusing the iterators and invoking undefined behavior. I couldn't find a clean way to simplify any of this with the <algorithm> library. Any tips on that would be great too.

Answer 1

The way you handle the first element as a special case is cumbersome: you're basically unrolling the first iteration of the loop. You could just initialize value = 0 so that the first XOR just copies the input to the output.

template <class InputIterator, class OutputIterator>
OutputIterator VisualEncrypt (InputIterator begin, InputIterator end, OutputIterator destination)
{
    typedef typename std::iterator_traits<InputIterator>::value_type T;
    T value = 0;
    while (begin != end) {
        value ^= *begin++;
        *destination++ = value ;
    }

    return destination ;
}

template <class InputIterator, class OutputIterator>
OutputIterator VisualDecrypt (InputIterator begin, InputIterator end, OutputIterator destination)
{
    typedef typename std::iterator_traits<InputIterator>::value_type T;
    T value = 0;

    while (begin != end) {
        T temp = *begin++;
        *destination++ = value ^ temp ;
        value = temp ;
    }

    return destination ;
}

Alternatively, use a for-loop, which results in slightly more compact code and gets the pesky side-effect ++ operators out of the way.

template <class InputIterator, class OutputIterator>
OutputIterator VisualEncrypt(InputIterator begin, InputIterator end, OutputIterator destination)
{
    typedef typename std::iterator_traits<InputIterator>::value_type T;
    for (T value = 0; begin != end; ++begin, ++destination) {
        *destination = (value ^= *begin);    // <-- Probably controversial one-liner
    }

    return destination;
}

template <class InputIterator, class OutputIterator>
OutputIterator VisualDecrypt(InputIterator begin, InputIterator end, OutputIterator destination)
{
    typedef typename std::iterator_traits<InputIterator>::value_type T;
    for (T value = 0; begin != end; ++begin, ++destination) {
        T temp = *begin;
        *destination = value ^ temp;
        value = temp;
    }

    return destination;
}

Answer 2

Taking @vnp's advice to use std::transform and @200_success's simplification, we can create two function objects.

An encrypt routine:

template <typename T>
struct Encrypt
{
    typedef T value_type ;

    value_type value ; 

    Encrypt () : value (0)
    {
    }

    value_type operator () (const value_type &input) {
        value ^= input ;
        return value ;
    }
};

And a decrypt routine:

template <typename T>
struct Decrypt
{
    typedef T value_type ;

    value_type value ; 

    Decrypt () : value (0)
    {
    }

    value_type operator () (const value_type &input) {
        value_type temp = value ;
        value = input ; 
        return temp ^ input ;
    }
};

The VisualEncrypt() and VisualDecrypt() functions then become trivial two-liners:

template <class InputIterator, class OutputIterator>
OutputIterator VisualEncrypt(InputIterator begin, InputIterator end, OutputIterator destination)
{
    typedef typename std::iterator_traits<InputIterator>::value_type value_type ;
    return std::transform (begin, end, destination, Encrypt <value_type> ()) ;
}

template <class InputIterator, class OutputIterator>
OutputIterator VisualDecrypt(InputIterator begin, InputIterator end, OutputIterator destination)
{
    typedef typename std::iterator_traits<InputIterator>::value_type value_type ;
    return std::transform (begin, end, destination, Decrypt <value_type> ()) ;
}

These encryption and decryption function signatures could be expanded to accept any encryption/decryption function objects:

template <class InputIterator, class OutputIterator, class EncryptOp>
OutputIterator VisualEncrypt(InputIterator begin, InputIterator end, OutputIterator destination, EncryptOp encrypt_op)
{
    return std::transform (begin, end, destination, encrypt_op) ;
}

template <class InputIterator, class OutputIterator, class DecryptOp>
OutputIterator VisualDecrypt(InputIterator begin, InputIterator end, OutputIterator destination, DecryptOp decrypt_op)
{
    return std::transform (begin, end, destination, decrypt_op) ;
}

But if you do that, you might as well just use std::transform() on its own.