Open Sv443 opened 3 years ago
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?
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.
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 :)
Alright, thanks for the response. I'll try implementing the size check.
its not causing a crash if you ensure that cryptedContent.size() % hash.size() == 0
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?
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());
}
I tried to use this but it's not crashing with the latest version of the code.
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
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.
So you mean that the key is larger than the actual decrypt text?
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);
}
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: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.
decode()
method or run by the user themselves prior to decoding if needed.decode()
method just throw a catch-able Error when the passed data is invalid.Technical Details: