nodemcu / nodemcu-firmware

Lua based interactive firmware for ESP8266, ESP8285 and ESP32
https://nodemcu.readthedocs.io
MIT License
7.61k stars 3.12k forks source link

Fix potential integer overflow in getnum #3633

Closed Crispy-fried-chicken closed 4 months ago

Crispy-fried-chicken commented 5 months ago

Fix the vulnerability mentioned in https://github.com/nodemcu/nodemcu-firmware/issues/3626

pjsg commented 5 months ago

This should be targeted at the dev branch. And we probably need the same fix in the dev-esp32 branch.

Crispy-fried-chicken commented 5 months ago

This should be targeted at the dev branch. And we probably need the same fix in the dev-esp32 branch.

I've changed the branch to dev, and as for dev-esp32, maybe I can create another PR?

pjsg commented 5 months ago

Yeah -- create another PR. Thanks

On Sun, Feb 4, 2024 at 10:05 PM Yiheng Cao @.***> wrote:

This should be targeted at the dev branch. And we probably need the same fix in the dev-esp32 branch.

I've changed the branch to dev, and as for dev-esp32, maybe I can create another PR?

— Reply to this email directly, view it on GitHub https://github.com/nodemcu/nodemcu-firmware/pull/3633#issuecomment-1926151533, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALQLTM5WJRXRRCVMBMIK43YSBD6LAVCNFSM6AAAAABCZMFNE2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRWGE2TCNJTGM . You are receiving this because you commented.Message ID: @.***>

Crispy-fried-chicken commented 5 months ago

and I see some checks were not successful, how I change to make it success?

Crispy-fried-chicken commented 5 months ago

I've already create another PR which is https://github.com/nodemcu/nodemcu-firmware/pull/3634, please check it, thank you!

pjsg commented 5 months ago

Hmm -- I have no idea why the checks failed..... I'm not sure who uses the 8266 dev branch....

HHHartmann commented 5 months ago

we need this fix on the release branch to fix the windows build issue: https://github.com/nodemcu/nodemcu-firmware/commit/193fe3593eb1537667179089535cdb7457327887#diff-5c3fa597431eda03ac3339ae6bf7f05e1a50d6fc7333679ec38e21b337cb6721

pjsg commented 5 months ago

Can you tee up a PR for that fix?

HHHartmann commented 5 months ago

Will do tomorrow

HHHartmann commented 5 months ago

Ah no, As it seems, you just need to rebase this branch onto a current dev branch which will also contain the fix.

Crispy-fried-chicken commented 5 months ago

@HHHartmann I don't see any fix about this PR in https://github.com/nodemcu/nodemcu-firmware/pull/3635, maybe you should add it to fix it?

HHHartmann commented 4 months ago

@Crispy-fried-chicken sorry, the fix on master is not required. I thought that the check pipleine needed to be updated on the release/master branch. But this is not needed. Rebasing this branch to dev is what should fix the checks as the correct definition is there.

Crispy-fried-chicken commented 3 months ago

@pjsg Hi, is it necessary to apply for a CVE for this vulnerability? This is very important to recognize our work, thank you!