AES
AES copied to clipboard
spaniakos
Last byte of plaintext should not be ignored.
Messages passed are byte arrays. There is no hint the array is NULL terminated string. Especially that length is given explicitly. Code should not assume final character is terminator in that case. Due to that substraction of 1 breaks the message (one byte of plaintext is ignored).
the -1 have to be replaced in more than one function. will implement the changes , thanks for the hint.
I checked the other functions in AES.cpp and don't think that there is any other function where the "-1" would need to be replaced. In my few this pull request can be accepted as is.
Only other potential change is the description of this function in AES.h which currently states "* @param p_size the size of the byte array ex sizeof(plaintext)". It might be useful to add a note that the sizeof() function may be incorrect in case of strings. For example byte plain[] = "0123456789" will have sizeof(plain) = 11 since it was created from a string, but strlen(plain) = 10 would be correct.
i will test the code. you are correct about string. the problem with the strings are that the \0 is calculated as a character. but since the library is supposed to handle bytes and not strings, it is better to have a function that will handle byte arrays and a separate function that will handle strings. the seconds function should have the -1 automatically when a string is given and should be a able to calculate the size on its own.
now for the padding, i saw that you you have made the padding array 0x01 to 0x10 that is also correct. as by using this kind of padding 0x10 = 16 the algorithm will be pkcs7 compliant. ( as it should have been from the beginning).
nevertheless, i will test your code against the tests and afterwards will accept the pull request.
Hi @spaniakos I've started to review the joint changes of #15 and #16 and plan to submit a pull in the next days. If you're interested, you can have a look at my branch "pad_fix" https://github.com/spaniakos/AES/compare/master...palto42:pad_fix (not tested yet).