Skip to content
New issue

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

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

Already on GitHub? # to your account

Only check the actual used length of the hash #8709

Merged
merged 1 commit into from
Nov 5, 2022

Conversation

seism0saurus
Copy link
Contributor

This fixes the error in #8707 but needs a professional review, since I am no C programmer.

The signature check fails, if the hash->len() is only 32 bytes instead of the assumed 64 bytes.
Since the variable HashLengthMax has a max in its name, I assume that it is allowed to have shorter hash lengths.
I assume that memcmp did an buffer over read here and read stuff after the 32 bytes of hash. But that's my narrow understanding of the internals of C as a non-C developer.

@mcspr mcspr linked an issue Nov 5, 2022 that may be closed by this pull request
6 tasks
@mcspr mcspr merged commit 80bf716 into esp8266:master Nov 5, 2022
@mcspr
Copy link
Collaborator

mcspr commented Nov 5, 2022

Thanks!

@mcspr
Copy link
Collaborator

mcspr commented Nov 5, 2022

I assume that memcmp did an buffer over read here and read stuff after the 32 bytes of hash.

Right. No function called to fill it (memset, std::fill), no zero-init (= {0};, = {};) or explicit member init (u8 buf[] {1,2,3}), so we end up with some random stack contents. Also, accidentally read 32bytes past hash object real buffer contents.

@d-a-v d-a-v changed the title Only check the actual used lenght of the hash. Only check the actual used length of the hash Jan 5, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ESPhttpUpdate Error[12] Signature verification failed
2 participants