LongSoft / UEFITool

UEFI firmware image viewer and editor
BSD 2-Clause "Simplified" License
4.47k stars 632 forks source link

Fix UINT32 underflow during NVRAM parsing (in NvramParser::parseEvsaStoreBody) #321

Closed yeggor closed 2 years ago

yeggor commented 2 years ago

This PR fixes UINT32 underflow in NvramParser::parseEvsaStoreBody. The issue occurs here: https://github.com/LongSoft/UEFITool/blob/5f134f783ad6d1e1cf58fc1133d40e64840b5950/common/nvramparser.cpp#L1716 calculateChecksum8 function takes as its second parameter a UINT32 bufferSize. Thus, if entryHeader->Size is 0 or 1, bufferSize will become MAX_UINT32 or (MAX_UINT32 - 1).

This will lead to OOB access inside calculateChecksum8 and segfault:

./build/UEFIExtract/UEFIExtract ~/tmp/2aa828cf6eaa4d5f9b7e86722838c97f40409fff082832a71c764a56662b31c4.bin

[1]    67234 segmentation fault  ./build/UEFIExtract/UEFIExtract

To fix this, I added an additional check here: https://github.com/LongSoft/UEFITool/blob/5f134f783ad6d1e1cf58fc1133d40e64840b5950/common/nvramparser.cpp#L1696

Just in case, I am attaching several files that cause this problem. uefitool_crash.zip

yeggor commented 2 years ago

Also, I think it makes sense to add a check here: https://github.com/LongSoft/UEFITool/blob/5f134f783ad6d1e1cf58fc1133d40e64840b5950/common/nvramparser.cpp#L1051

NikolajSchlej commented 2 years ago

Yeah, that NVRAM parser is in a dire need of Kaitai treatment, i.e. getting replaced by an autogenerated parser based on KS DSL.

I'll merge the PR once CI is done.

yeggor commented 2 years ago

It would definitely be great to have for NVRAM what you did with BootGuard/ACM related structures.