Fix double free vulnerability

src/Cipher.cpp

@@ -7,59 +7,55 @@
 
 #include "Cipher.h"
 
+#define CIPHER_DEFAULT_KEY "abcdefghijklmnop"
+
 Cipher::Cipher() {
   // default unsecure key, its highly recommended to use the overloaded constructor and the function setKey()
-  // sometimes serval keys wont work 
+  // sometimes serval keys wont work
   // https://tls.mbed.org/kb/how-to/generate-an-aes-key
-
-  setKey("abcdefghijklmnop");
-}
 
-Cipher::Cipher(char * key) {
-	setKey(key);
+  setKey(CIPHER_DEFAULT_KEY);
 }
 
-Cipher::~Cipher() {
-	delete privateCipherKey;
+Cipher::Cipher(const char * key) {
+	setKey(key);
 }
 
-void Cipher::setKey(char * key) {
-  // aes-128bit mode means that your cipher key can only be 16 characters long 
+void Cipher::setKey(const char * key) {
+  // aes-128bit mode means that your cipher key can only be 16 characters long
   // futhermore, only chracters in the cipher key are allowed, not numbers!
   // 16 characters + '\0'
-  
+
   if( strlen(key) > 16 ) {
-    privateCipherKey = new char[17];
-    (String(key).substring(0,16)).toCharArray(privateCipherKey, 17);
-
+    String(key).substring(0,16).toCharArray(privateCipherKey, 17);
+
     #ifdef CIPHER_DEBUG
       Serial.println("[cipher] error: cipher key to long! Will be cutted to 16 characters.");
       Serial.println("[cipher] => " + String(key));
       Serial.println("[cipher] => " + String(privateCipherKey));
-    #endif  
+    #endif
   } else if( strlen(key) < 16 ) {
-    privateCipherKey = "abcdefghijklmnop";
-    
+    strcpy(privateCipherKey, CIPHER_DEFAULT_KEY);
+
     #ifdef CIPHER_DEBUG
       Serial.println("[cipher] error: cipher key to short! Standard cipher key will be used.");
     #endif
   } else {
     #ifdef CIPHER_DEBUG
       Serial.println("[cipher] cipher key length matched. Using this key.");
     #endif
-    privateCipherKey = key;
+    strcpy(privateCipherKey, key);
   }
 }
 
 char * Cipher::getKey() {
   return privateCipherKey;
 }
 
-
 void Cipher::encrypt(char * plainText, char * key, unsigned char * outputBuffer) {
   // encrypt plainText buffer of length 16 characters
   mbedtls_aes_context aes;
- 
+
   mbedtls_aes_init( &aes );
   mbedtls_aes_setkey_enc( &aes, (const unsigned char*) key, strlen(key) * 8 );
   mbedtls_aes_crypt_ecb( &aes, MBEDTLS_AES_ENCRYPT, (const unsigned char*)plainText, outputBuffer);
@@ -73,7 +69,7 @@ void Cipher::encrypt(char * plainText, unsigned char * outputBuffer) {
 void Cipher::decrypt(unsigned char * cipherText, char * key, unsigned char * outputBuffer) {
   // encrypt ciphered chipherText buffer of length 16 characters to plain text
   mbedtls_aes_context aes;
- 
+
   mbedtls_aes_init( &aes );
   mbedtls_aes_setkey_dec( &aes, (const unsigned char*) key, strlen(key) * 8 );
   mbedtls_aes_crypt_ecb(&aes, MBEDTLS_AES_DECRYPT, (const unsigned char*)cipherText, outputBuffer);
@@ -89,10 +85,10 @@ String Cipher::encryptBuffer(char * plainText, char * key) {
   // returns encrypted String of plainText (length: 16 characters)
   String cipherTextString = "";
   unsigned char cipherTextOutput[16];
-  
+
   encrypt(plainText, key, cipherTextOutput);
-  
-  for (int i = 0; i < 16; i++) {
+
+  for (int8_t i = 0; i < 16; i++) {
     cipherTextString = cipherTextString + (char)cipherTextOutput[i];
   }
 
@@ -109,13 +105,13 @@ String Cipher::decryptBuffer(String cipherText, char * key) {
   unsigned char cipherTextOutput[16];
   unsigned char decipheredTextOutput[16];
 
-  for (int i = 0; i < 16; i++) {
+  for (int8_t i = 0; i < 16; i++) {
     cipherTextOutput[i] = (char)cipherText[i];
   }
-  
+
   decrypt(cipherTextOutput, key, decipheredTextOutput);
 
-  for (int i = 0; i < 16; i++) {
+  for (int8_t i = 0; i < 16; i++) {
     decipheredTextString = decipheredTextString + (char)decipheredTextOutput[i];
 
     if(decipheredTextString[i] == '\0') {
@@ -136,20 +132,20 @@ String Cipher::encryptString(String plainText, char * key) {
   constexpr int BUFF_SIZE=16;
   String buffer = "";
   String cipherTextString = "";
-  int index = plainText.length() / BUFF_SIZE;
-  
+  const int index = plainText.length() / BUFF_SIZE;
+
   for(int block=0; block < plainText.length()/BUFF_SIZE; block++) {
       for(int j = block*BUFF_SIZE; j < (block+1)*BUFF_SIZE; j++) {
         buffer += plainText[j];
       }
-      
+
       cipherTextString += encryptBuffer(const_cast<char*>(buffer.c_str()), key);
       buffer = "";
   }
 
   buffer="";
 
-  if( plainText.length()%BUFF_SIZE > 0 ) {    
+  if( plainText.length()%BUFF_SIZE > 0 ) {
     for(int bytes_read=(index*BUFF_SIZE); bytes_read <= (index*BUFF_SIZE) + plainText.length()%BUFF_SIZE; bytes_read++) {
       buffer += plainText[bytes_read];
     };
@@ -168,12 +164,12 @@ String Cipher::decryptString(String cipherText, char * key) {
   constexpr int BUFF_SIZE=16;
   String buffer = "";
   String decipheredTextString = "";
-  
+
   for(int block=0; block < cipherText.length()/BUFF_SIZE; block++) {
       for(int j = block*BUFF_SIZE; j < (block+1)*BUFF_SIZE; j++) {
         buffer += cipherText[j];
       }
-      
+
       decipheredTextString += decryptBuffer(buffer, key);
       buffer = "";
   }
@@ -183,4 +179,4 @@ String Cipher::decryptString(String cipherText, char * key) {
 
 String Cipher::decryptString(String cipherText) {
   return decryptString(cipherText, getKey());
-}
+}

src/Cipher.h

@@ -12,8 +12,6 @@
 #include <Arduino.h>
 //#include <WString.h>
 
-#define CIPHER_DEBUG
-
 class Cipher {
 public:
   /** Default constructor, privateChiperKey property will be set on a default, unsecure value
@@ -22,65 +20,65 @@ class Cipher {
    *  @return ---
   */
   Cipher();
-  
+
   /** Overloaded constructor, privateChiperKey will be set on @param key
    *
    *  @param key secure key as pointer on char array
    *  @return ---
   */
-	Cipher(char * key);
+	Cipher(const char * key);
 
-  /** Default destructor, privateChiperKey will deleted
+  /** Default destructor
    *
    *  @param  ---
    *  @return ---
   */
-  virtual ~Cipher();
+  ~Cipher() = default;
 
 
   /** Set privateChiperKey on given @param key
    *
    *  @param key secure key as pointer on char array
    *  @return ---
   */
-  void setKey(char * key);
+  void setKey(const char * key);
 
   /** Returns the privateCipherKey
    *
    *  @param  ---
    *  @return secure key as pointer on char array
-  */  
-  char * getKey();
+  */
+  const char * getKey();
 
 
   /** Encrypt (AES-128bit ECB encryption mode) the @param plainText char array with given key and store the output in @return outputBuffer
    *
    *  @param plainText buffer of length 16 characters
    *  @param key secure key as pointer on char array, function call getKey() is possible, instead of using the second function encrypt()
    *  @return outputBuffer buffer of length 16 characters filled with the encryption result
-  */ 
+  */
 	void encrypt(char * plainText, char * key, unsigned char * outputBuffer);
 
   /** Encrypt (AES-128bit ECB encryption mode) the @param plainText char array with @property privateCipherKey and store the output in @return outputBuffer
    *
    *  @param plainText buffer of length 16 characters
    *  @return outputBuffer buffer of length 16 characters filled with the encryption result
-  */ 
+  */
   void encrypt(char * plainText, unsigned char * outputBuffer);
 
   /** Decrypt (AES-128bit ECB decryption mode) the @param cipherText char array with given key and store the output in @return outputBuffer
    *
    *  @param cipherText buffer of length 16 characters
    *  @param key secure key as pointer on char array, function call getKey() is possible, instead of using the second function decrypt()
    *  @return outputBuffer buffer of length 16 characters filled with the decryption result
-  */  
+  */
   void decrypt(unsigned char * cipherText, char * key, unsigned char * outputBuffer);
 
   /** Decrypt (AES-128bit ECB decryption mode) the @param cipherText char array with @property privateCipherKey and store the output in @return outputBuffer
    *
    *  @param cipherText buffer of length 16 characters
    *  @return outputBuffer buffer of length 16 characters filled with the decryption result
-  */  
+  */
   void decrypt(unsigned char * cipherText, unsigned char * outputBuffer);
 
 
@@ -144,9 +142,9 @@ class Cipher {
    *  @return decipheredTextString String of length 16 characters filled with the decryption result
   */
   String decryptString(String cipherText);
-  
+
 private:
-	char * privateCipherKey;
+	char privateCipherKey[17];
 };
 
-#endif /* CIPHER_H_ */
+#endif /* CIPHER_H_ */