suculent / thinx-aes-lib

AES wrapper for ESP8266/ESP32/Arduino/nRF5x
Other
113 stars 39 forks source link

Error with Simple example running on Arduino Nano #34

Closed datedave closed 3 years ago

datedave commented 3 years ago

Using an Arduino Nano, with the "simple" example provided, the decrkypt fails (output below) - any ideas?

because the decrypt looks almost there, I wonder if there is some buffer over write somewhere?

readBuffer length: 19 Calling encrypt (string)... Encrypted length = 44 Encrypted. Decrypting... 44 Calling decrypt...; Decrypted bytes: 32 Decrypted cleartext of length: 32 Decrypted cleartext: username:passwor⸮s⸮lڶ--⸮⸮n=⸮ Decryption test failed.

suculent commented 3 years ago

This seems more like the aes_iv changes on encryption and forgot being reset to same initial state on decryption...

On 17 Sep 2020, at 15:54, datedave notifications@github.com wrote:

Using an Arduino Nano, with the "simple" example provided, the decrkypt fails (output below) - any ideas?

because the decrypt looks almost there, I wonder if there is some buffer over write somewhere?

readBuffer length: 19 Calling encrypt (string)... Encrypted length = 44 Encrypted. Decrypting... 44 Calling decrypt...; Decrypted bytes: 32 Decrypted cleartext of length: 32 Decrypted cleartext: username:passwor⸮s⸮lڶ--�⸮�⸮n=⸮ Decryption test failed.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/suculent/thinx-aes-lib/issues/34, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABWFRY3Z5AQ6UXELGFYKNTSGIIK7ANCNFSM4RQPI54A.

datedave commented 3 years ago

Ok, so I think I found the error with you example... but you will need to check as I am no expert!

The call to aesLib.encrypt() writes the cypher data back to the same buffer that ir read the clear data from. This causes a problem if the cypher data is longer than the clear data.

So looking at the error above, readbuffer was length 19, but Encrypted was length 44, hence a buffer over write.

Not being familiar with the maths behind AES encryption, but assuming there is always a risk that the encrypted data could be longer than the clear data, I suggest its better to return a new newly allocated buffer with the encrypted data, rather than ovwe write??

hope that helps

datedave commented 3 years ago

to check the above I increased the size of the readBuffer to longer that the input string..

i.e. unsigned char readBuffer[200] = "username:password";

and got the output readBuffer length: 200 Calling encrypt (string)... Encrypted length = 280 Encrypted. Decrypting... 280 Calling degth = 280 Encrypted. Decrypting... 280 lling decDecrypted bytes: 208 Decrypted cleartext of length: 208 Decrypted cleartext: username:password Decrypted correctly.

Although that could have been a fluke.

datedave commented 3 years ago

Again though it is still returning a length bigger than the input - so assuming you are re-allocating buffer internal to lib?

datedave commented 3 years ago

Digging a bit deeper.. I think my fix above was a fluke.. I can see now that you return the txt into a differnt global buffer

suculent commented 3 years ago

Well, I'm trying to solve it but probably will have to back to it later with better focus.

I'm just refactoring some another crazy project...

On 17 Sep 2020, at 16:17, datedave notifications@github.com wrote:

Digging a bit deeper.. I think my fix above was a fluke.. I can see now that you return the txt into a differnt global buffer

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/suculent/thinx-aes-lib/issues/34#issuecomment-694267319, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABWFRY7EFAZ4CGU35XQC5TSGIK77ANCNFSM4RQPI54A.

datedave commented 3 years ago

Thanks - much appreciated. I'll have a look around myself, but this is probably way out of my league.

suculent commented 3 years ago

I don't think so. This is actually a beginner's termination error in C-string. I'm sometimes repeating those even after 20 years...

On 17 Sep 2020, at 16:25, datedave notifications@github.com wrote:

Thanks - much appreciated. I'll have a look around myself, but this is probably way out of my league.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/suculent/thinx-aes-lib/issues/34#issuecomment-694272106, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABWFR6LF6G2JH5SSYNIAGDSGIL35ANCNFSM4RQPI54A.

datedave commented 3 years ago

If I understand you, you are saying that somewhere you are not adding the null '\0' to terminate the C str correctly - right?

Or are you not allowing enough space at the end of a buffer for a '\0' somewhere?

irineu commented 3 years ago

getting the same problem here

suculent commented 3 years ago

There has been issue with missing aesLib.set_paddingmode(0) in examples.