Suggestion: Function to check validity of encoded data OR make the decode() method not crash app on invalid data

[Sv443]

I'm currently at a point that requires me to decode an encrypted QByteArray but in my case it is possible for the passed data to be invalid.
If I now pass this invalid data through the decode() method the entire app crashes, giving me the following error:

HEAP[MyApp.exe]: Heap block at 000001DA7A1C7B10 modified at 000001DA7A1C7B70 past requested size of 50
HEAP[MyApp.exe]: Invalid address specified to RtlValidateHeap( 000001DA706F0000, 000001DA7A1C7B20 )
Debug Assertion Failed!

Program: ...sktop_Qt_5_12_8_MSVC2017_64bit-Debug\debug\MyApp.exe
File: minkernel\crts\ucrt\src\appcrt\heap\debug_heap.cpp
Line: 904

Expression: _CrtIsValidHeapPointer(block)

Using a try {} catch {} doesn't work, since the error seems to corrupt the heap in some way.
I know I should fix the source of the problem rather than trying to fix the symptoms but I don't see a bulletproof way to do this in my case.

• My suggestion is to add a function / method that checks the validity of a passed (encoded) QByteArray, which can either directly be implemented in the decode() method or run by the user themselves prior to decoding if needed.
• An alternative solution is to make the decode() method just throw a catch-able Error when the passed data is invalid.

Technical Details:

• Qt v5.14.2
• Qt-AES v1.1
• OS: Windows 10 (1909)
• Compiler: MSVC 2017 64bit

[bricke]

Hi, definitely I should add some better error handling, in the meanwhile, on your side, you could check the size of your data just prior of calling the decode.

Does it make sense?

[Sv443]

Yeah I can do that as a coarse filter, but that doesn't completely eliminate the problem.
I'd volunteer to help implement the error checking but your code goes way over my head.

[bricke]

Size is the only factor that could result in such crash, I am sure if you do a size check we can avoid disaster, I will definitely put some better error handling on the pile of improvements that I should do :)

[Sv443]

Alright, thanks for the response. I'll try implementing the size check.

[jebos]

its not causing a crash if you ensure that cryptedContent.size() % hash.size() == 0

[bricke]

Hi, Sv443, I have tried passing invalid text to the decode function and it's not crashing.
Can you share an example of bad data?

[Sv443]

Unfortunately I am no longer involved with the project that used this library, but this is the code I used and if I recall correctly, passing any invalid string caused a crash:

QAESEncryption m_encryption = new QAESEncryption(QAESEncryption::Aes::AES_128, QAESEncryption::Mode::ECB);

QString Crypto::encrypt(QString str, QString key)
{
    QByteArray encodedText = m_encryption.encode(str.toLatin1(), key.toLatin1());

    return QString(encodedText.toBase64());
}

[bricke]

I tried to use this but it's not crashing with the latest version of the code.

[Sv443]

That's great, it must've been something specific to our project then, but since I'm no longer working on it I can't really pursue this any further

[jebos]

I Have to check with most latest code.

But basically i get a heap corruption in case i pass ie "abcd" in decrypt with some lager hash value.

[bricke]

So you mean that the key is larger than the actual decrypt text?

[a379933101]

Checkthe string is a Base64 string andthe length is a multiple of 16 after decoding(just for aes128)!!
std::string NavStringUtils::aesCBCPKCS5PADDINGDecode(const std::string& pcInput,const std::string& key, const std::string& iv){
int strLen = pcInput.length();

// isbase64 str
for(int i=0;i<strLen; i ++){
    char c = pcInput[i];
    if(!is_base64(c) && c != '=')return "";
}

// is aes bytes
QByteArray dataDecode = QByteArray::fromBase64(pcInput.c_str());
if(dataDecode.length() % 16 != 0){
    return "";
}

QAESEncryption encryption(QAESEncryption::AES_128, QAESEncryption::CBC,QAESEncryption::PKCS7);

// printf("dataDecodedataDecodedataDecodedataDecode len = %ld \n",dataDecode.length());
QByteArray decodeData = encryption.decode(dataDecode,key.c_str(),iv.c_str());
int size = decodeData.length();
if(size == 0)return "";
int removeNum = (int)decodeData[size - 1];
char* data = decodeData.remove(size - removeNum , removeNum).data();

return std::string(data);

}