Synergy: Integer Overflow, Key Reuse, IV Reuse

Synergy is a software that lets you share your mouse and keyboard between multiple computers. Until recently, it didn't have any support for encryption, which left users' keystrokes and mouse movements vulnerable to sniffing on the local network. Users worked around this limitation by tunneling Synergy's communications through a VPN or SSH. Lots of users complained, and the Synergy team decided to encrypt the connections. Instead of using a well-known protocol like TLS, they invented their own.

The most obvious problem is that they are re-using the same key and IV for encrypted communications in both directions. This is absolutely fatal for modes like CTR, OFB, GCM, and probably CFB, which are the ones Synergy supports. Update: A tool that exploits this vulnerability is available here.

Second, they derive the IV from the password, and because of an integer overflow bug, when the length of the password is congruent to its double mod 256, the key and the IV are the same. If the IV were the same size as the output of the hash function they're using (SHA256), you'd be able to recover the key for all passwords longer than 128 characters.

 if (!options.m_pass.empty()) {  
     createKey(m_key, options.m_pass, kKeyLength, static_cast<UInt8>(options.m_pass.length()));  
     byte iv[CRYPTO_IV_SIZE];  
     createKey(iv, options.m_pass, CRYPTO_IV_SIZE, static_cast<UInt8>(options.m_pass.length()) * 2);  
     setEncryptIv(iv);  
     setDecryptIv(iv);  
  }  

 void  
 CCryptoStream::createKey(byte* out, const CString& password, UInt8 keyLength, UInt8 hashCount)  
 {  
      assert(keyLength <= SHA256::DIGESTSIZE);  
      byte temp[SHA256::DIGESTSIZE];  
      byte* in = reinterpret_cast<byte*>(const_cast<char*>(password.c_str()));  
      SHA256().CalculateDigest(temp, in, password.length());  
      byte* tempKey = new byte[SHA256::DIGESTSIZE];  
      for (int i = 0; i < hashCount; ++i) {  
           memcpy(tempKey, temp, SHA256::DIGESTSIZE);  
           SHA256().CalculateDigest(temp, tempKey, SHA256::DIGESTSIZE);  
      }  
      delete[] tempKey;  
      memcpy(out, temp, keyLength);  
 }  

There is no attempt to detect or prevent replay attacks, and except for GCM mode, there is no authentication or data integrity guarantees (which you really want in something that's accepting keyboard commands).

The developers seem to be aware of how bad their crypto is, but for some reason refuse to fix or disable it.

Lessions learned:

• Never invent your own network encryption protocols. Even professional cryptographers can't get it right the first time (SSL 3.0, TLS 1.0). Always use a good TLS library.
• Always generate a random IV at the time of encryption.
• Don't use a key for anything other than a key. All other uses will leak information about it.
• It's better to have no encryption than to have broken encryption with a false sense of security.