From ea6f020f35069a3aaff761a4e7b769d5fba9dfbe Mon Sep 17 00:00:00 2001 From: Reg <69511985+RegularRabbit05@users.noreply.github.com> Date: Sat, 6 Dec 2025 15:12:37 +0100 Subject: [PATCH 1/2] Fix double free vulnerability --- src/Cipher.cpp | 64 +++++++++++++++++++++++--------------------------- src/Cipher.h | 28 ++++++++++------------ 2 files changed, 43 insertions(+), 49 deletions(-) diff --git a/src/Cipher.cpp b/src/Cipher.cpp index 3ff86a7..c13d376 100644 --- a/src/Cipher.cpp +++ b/src/Cipher.cpp @@ -7,39 +7,36 @@ #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 @@ -47,7 +44,7 @@ void Cipher::setKey(char * key) { #ifdef CIPHER_DEBUG Serial.println("[cipher] cipher key length matched. Using this key."); #endif - privateCipherKey = key; + strcpy(privateCipherKey, key); } } @@ -55,11 +52,10 @@ 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(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()); -} +} \ No newline at end of file diff --git a/src/Cipher.h b/src/Cipher.h index 7820264..12ac352 100644 --- a/src/Cipher.h +++ b/src/Cipher.h @@ -12,8 +12,6 @@ #include //#include -#define CIPHER_DEBUG - class Cipher { public: /** Default constructor, privateChiperKey property will be set on a default, unsecure value @@ -22,20 +20,20 @@ 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 @@ -43,13 +41,13 @@ class Cipher { * @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(); @@ -58,14 +56,14 @@ class Cipher { * @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 @@ -73,14 +71,14 @@ class Cipher { * @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_ */ \ No newline at end of file From 09f6f66d3459e4e8b170f0197e5f88b3622d98a2 Mon Sep 17 00:00:00 2001 From: Reg <69511985+RegularRabbit05@users.noreply.github.com> Date: Sat, 6 Dec 2025 15:23:40 +0100 Subject: [PATCH 2/2] Change function return type Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/Cipher.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Cipher.h b/src/Cipher.h index 12ac352..b2e5c19 100644 --- a/src/Cipher.h +++ b/src/Cipher.h @@ -48,7 +48,7 @@ class Cipher { * @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