Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Array ota_info.md5 out of boundaries on OTA when calling base64_decode #67

Open
bastianhjaeger opened this issue Feb 12, 2019 · 2 comments

Comments

@bastianhjaeger
Copy link

bastianhjaeger commented Feb 12, 2019

Version Information:

  • ESP8266MQTTMesh version:
  • AsyncMQTTClient version: 0.8.2
  • ESPAsyncTCP version: 1.2.0
  • ESP8266 Core version: commit 3745054
  • PlatformIO: version 4.0.0a3
  • /.platformio/platforms/espressif8266 commit: 7b360716161fa31175ba9faca7f4e7d6960a584c

Description of problem:
Probable overflow of ota_info.md5 on
https://github.com/PhracturedBlue/ESP8266MQTTMesh/blob/master/src/ESP8266MQTTMesh.cpp#L754
in function base64_decode(..)
https://github.com/PhracturedBlue/ESP8266MQTTMesh/blob/master/src/Base64.cpp#L59

I just tested it and, according to my tests, decLen goes up to 16. Thus, ota_info.md5 goes out of it boundaries.

@simone1999
Copy link
Collaborator

looks like you are absolutely right, I think the Problem is that the decoded message is in fact 16 byte long, which is absolutely right, but then at the End of the Function the String Terminator (\0) gets addet, which is the 17th byte and like you said this is overflowing.

@simone1999
Copy link
Collaborator

one verry interesting thing is also, that the Function base64_decode also gets used to decode the whole Firmware sent over the OTA function. This means that every Block of Memmory has a String Terminator (B00000000) at the End, maybe the Instruction (B00000000) doesn't do anything (maybe the NOP Instruction) but it defently gets written in the Flash! I'm thinking about just removing the String Terminator in the base64_decode, because if you really want to use String Functions on the decoded CharArray, you could just add the Terminator after you called base64_decode. besides the only 2 times the Function is cadded is for decoding the MD5 sum and decoding the OTA binary packets. And base64_encode isn't called at all

simone1999 added a commit to simone1999/ESP8266MQTTMesh that referenced this issue Jul 26, 2019
String Terminator was a verry bad Idea, because it causes an Overflow in decoding the MD5 Sum and writing String Terminators in the executed Flash by an OTA Update
see PhracturedBlue#67
simone1999 added a commit to simone1999/ESP8266MQTTMesh that referenced this issue Jul 26, 2019
String Terminator was a verry bad Idea, because it causes an Overflow in decoding the MD5 Sum and writing String Terminators in the executed Flash by an OTA Update
see PhracturedBlue#67
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants