Error C6385 in C++ (AES)

[Lennart van de Merwe]

unsigned char* decryptedmessage = new unsigned char[LenOfPaddedMessage];
        for (int i = 0; i < LenOfPaddedMessage; i++) {
            decryptedmessage[i] = encryptedmessage2[i];
        }

        for (int i = 0; i < LenOfPaddedMessage; i += 16) {
            AES_Decrypt(decryptedmessage + i, inputkey);

        }

        cout << endl << endl;
        cout << "\nDecrypted message:" << endl;
        for (int i = 0; i < LenOfPaddedMessage; i++)
        {
            cout << decryptedmessage[i];
            
        }
        cout << endl << endl;
        
        string AnswerYesNo; // een test/encryptie module
        cout << "\nType 'yes' if you wish to encrypt the message, answer 'no' if not: ";
        getline(cin, AnswerYesNo);
        int AnswerLength;
        AnswerLength = AnswerYesNo.length();

        if (AnswerLength == 3)
        {
            int originalLen = strlen((const char*)decryptedmessage);
            int lenOfPaddedMessage = originalLen;

            if (lenOfPaddedMessage % 16 != 0)
                lenOfPaddedMessage = (lenOfPaddedMessage / 16 + 1) * 16;

            unsigned char* paddedMessage = new unsigned char[lenOfPaddedMessage];
            for (int x = 0; x < lenOfPaddedMessage; x++)
            {
                if (x >= originalLen) paddedMessage[x] = 0;
                else paddedMessage[x] = decryptedmessage[x];
            }

Hi guys this is Austin, I'm having a problem with my C++ project for school (it's making AES, if you're wondering). I'm a massive noob when it comes to coding, let alone C languages, so please have mercy on my soul.

The problem I'm having is in the second to last line: "... else paddedMessage[x] = decryptedmessage[x];" 
The warning Visual gives me is coded C6385. Of course I've looked this up and I understand what the problem might be, but I can't seem to figure out the cause of it. I suspect it's something to do "decryptedmessage", but I'm not sure.The code listed above isn't the entire code (that's something like 600 lines: too much for this post I suppose, but if you need it, just ask), I've chosen to take the code from the 'birth' of decryptedmessage until the error. If you need more, once again, just ask.

I'm sorry for my awful description of the problem and I know I'm a horrible noob. Please help me out, it would mean a lot.
Thanks in advance.

[AbydosOne]

9 minutes ago, Lennart van de Merwe said:

coded C6385

See here.

 

Basically, it's just warning you there's a chance that 'x' might be a larger value than the declared length of 'decryptedMessage'. Check that x < decryptedMessageLength in your for loop.

[Lennart van de Merwe]

14 minutes ago, AbydosOne said:

See here.

 

Basically, it's just warning you there's a chance that 'x' might be a larger value than the declared length of 'decryptedMessage'. Check that x < decryptedMessageLength in your for loop.

I'm sorry, but what do you mean by that? I'm a noob remember X<

[AbydosOne]

59 minutes ago, Lennart van de Merwe said:

I'm sorry, but what do you mean by that? I'm a noob remember X<

Basically, if you have something with 10 slots, and you ask it for whatever is in slot 11, the thing isn't going to know how to respond, and so it might throw an error, or crash, or give you what would be in slot 11 if it existed (this is usually the nastiest outcome for various reasons).

Point is, there a chance you might ask the array for something in a position that doesn't exist, and so you need to ensure that your value of 'x' doesn't tell the arrays 'decryptedmessage' or 'paddedMessage' (in that line) to pick from a slot beyond what they have, by making sure that 'x' is always smaller than the length of either array.

[Lennart van de Merwe]

2 minutes ago, AbydosOne said:

Basically, if you have something with 10 slots, and you ask it for whatever is in slot 11, the thing isn't going to know how to respond, and so it might throw an error, or crash, or give you what would be in slot 11 if it existed (this is usually the nastiest outcome for various reasons).

Point is, there a chance you might ask the array for something in a position that doesn't exist, and so you need to ensure that your value of 'x' doesn't tell the arrays 'decryptedmessage' or 'paddedMessage' (in that line) to pick from a slot beyond what they have, by making sure that 'x' is always smaller than the length of either array.

Yes, but that int is determined by taking the length of the decryptedmessage, which then also defines the length of the paddedmessage. Could it be something to do with:
 

int originalLen = strlen((const char*)decryptedmessage);
			int lenOfPaddedMessage = originalLen;

[wanderingfool2]

5 minutes ago, Lennart van de Merwe said:

Yes, but that int is determined by taking the length of the decryptedmessage, which then also defines the length of the paddedmessage. Could it be something to do with:
 

int originalLen = strlen((const char*)decryptedmessage);
			int lenOfPaddedMessage = originalLen;

It's a compiler warning, which means that it doesn't always do the most indepth analysis of what the upper bound may be.  It is giving that warning because it just detected that the number might be larger than the original (in this case it is sort of a guideline to look into to make sure it won't).

 

As a note though, strlen is null terminated; so yes originalLen could be greater than the defined length and you could be reading the memory that comes after the decryptedmessage.

int main() {
	char[2] a;
    a[0] = 'a';
    a[1] = 'b';
    int len = strlen(a); //Len is not well defined.  In theory you might think it is 2, but in practice it could be anything
    	//Since it will depend what is stored in the memory location a[2] (sometimes it might be 0, so you might get the right answer)
        //but it isn't guarenteed
	return 0;
}

 

All you could use something like as follows to catch it properly

if(lenOfPaddedMessage > LenOfPaddedMessage)
	lenOfPaddedMessage = LenOfPaddedMessage;

 

*So now this is just my opinion, but it I always try keeping variable names distinct from each other.  Yes, l vs L makes it a different variable but to me it just makes things confusing when they are used together (having to remember which is which as I don't usually process the capitals in my head)

[Lennart van de Merwe]

10 minutes ago, wanderingfool2 said:

It's a compiler warning, which means that it doesn't always do the most indepth analysis of what the upper bound may be.  It is giving that warning because it just detected that the number might be larger than the original (in this case it is sort of a guideline to look into to make sure it won't).

 

As a note though, strlen is null terminated; so yes originalLen could be greater than the defined length and you could be reading the memory that comes after the decryptedmessage.

int main() {
	char[2] a;
    a[0] = 'a';
    a[1] = 'b';
    int len = strlen(a); //Len is not well defined.  In theory you might think it is 2, but in practice it could be anything
    	//Since it will depend what is stored in the memory location a[2] (sometimes it might be 0, so you might get the right answer)
        //but it isn't guarenteed
	return 0;
}

 

All you could use something like as follows to catch it properly

if(lenOfPaddedMessage > LenOfPaddedMessage)
	lenOfPaddedMessage = LenOfPaddedMessage;

 

*So now this is just my opinion, but it I always try keeping variable names distinct from each other.  Yes, l vs L makes it a different variable but to me it just makes things confusing when they are used together (having to remember which is which as I don't usually process the capitals in my head)

Thanks a lot for the input and clear explanation. Unfortunately it only makes it worse when it comes to the output of the code, so I'm leaving the bug as it is, especially since I have to start handing in my assignment.

[wanderingfool2]

17 minutes ago, Lennart van de Merwe said:

Thanks a lot for the input and clear explanation. Unfortunately it only makes it worse when it comes to the output of the code, so I'm leaving the bug as it is, especially since I have to start handing in my assignment.

Okay, so I actually read your code...yea, you would actually want to apply the fix to originalLen...or if it allows it strnlen...and cap it there