From cf01543f06863b5921bd9ab749dc460b84f2d4df Mon Sep 17 00:00:00 2001 From: vit9696 Date: Tue, 8 May 2018 18:42:16 +0300 Subject: [PATCH] Silence analyzer warnings and fix potential issues --- .gitignore | 7 ++++++ UEFITool/hexviewdialog.cpp | 8 +++---- UEFITool/uefitool.cpp | 4 ++-- common/ffsops.cpp | 8 +------ common/ffsparser.cpp | 33 +++++++++++++------------- common/ffsparser.h | 2 +- common/nvramparser.cpp | 47 +++++++++++++++++--------------------- common/nvramparser.h | 2 +- common/parsingdata.h | 8 +++---- common/peimage.h | 4 ++-- common/utility.cpp | 35 ++++++++++++++++++++-------- 11 files changed, 85 insertions(+), 73 deletions(-) diff --git a/.gitignore b/.gitignore index 1a4afc2..9eb95c6 100644 --- a/.gitignore +++ b/.gitignore @@ -231,3 +231,10 @@ pip-log.txt ############# *.o Makefile + +UEFITool/uefitool_plugin_import.cpp +UEFITool.app/ +.qmake.stash +CMakeCache.txt +CMakeFiles +cmake_install.cmake diff --git a/UEFITool/hexviewdialog.cpp b/UEFITool/hexviewdialog.cpp index e1d217f..11e6ed1 100644 --- a/UEFITool/hexviewdialog.cpp +++ b/UEFITool/hexviewdialog.cpp @@ -47,8 +47,8 @@ void HexViewDialog::setItem(const UModelIndex & index, bool bodyOnly) setWindowTitle(UString("Hex view: ") + (itemText.isEmpty() ? itemName : itemName + " | " + itemText)); // Set hex data - QByteArray data; - if (bodyOnly) data = model->body(index); - else data = model->header(index) + model->body(index) + model->tail(index); - hexView->setData(data); + QByteArray hexdata; + if (bodyOnly) hexdata = model->body(index); + else hexdata = model->header(index) + model->body(index) + model->tail(index); + hexView->setData(hexdata); } \ No newline at end of file diff --git a/UEFITool/uefitool.cpp b/UEFITool/uefitool.cpp index 9721663..6659c25 100644 --- a/UEFITool/uefitool.cpp +++ b/UEFITool/uefitool.cpp @@ -374,8 +374,8 @@ void UEFITool::goToData() if (model->hasEmptyParsingData(index)) continue; - UByteArray data = model->parsingData(index); - const NVAR_ENTRY_PARSING_DATA* pdata = (const NVAR_ENTRY_PARSING_DATA*)data.constData(); + UByteArray rdata = model->parsingData(index); + const NVAR_ENTRY_PARSING_DATA* pdata = (const NVAR_ENTRY_PARSING_DATA*)rdata.constData(); UINT32 lastVariableFlag = pdata->emptyByte ? 0xFFFFFF : 0; UINT32 offset = model->offset(index); if (pdata->next == lastVariableFlag) { diff --git a/common/ffsops.cpp b/common/ffsops.cpp index 6f94039..4ef2b8b 100644 --- a/common/ffsops.cpp +++ b/common/ffsops.cpp @@ -21,10 +21,6 @@ USTATUS FfsOperations::extract(const UModelIndex & index, UString & name, UByteA if (!index.isValid()) return U_INVALID_PARAMETER; - // Construct a name for extracted data - UString itemName = model->name(index); - UString itemText = model->text(index); - // Default name name = uniqueItemName(index); @@ -77,10 +73,8 @@ USTATUS FfsOperations::replace(const UModelIndex & index, const UString & data, else if (mode == REPLACE_MODE_BODY) { return U_NOT_IMPLEMENTED; } - else - return U_UNKNOWN_REPLACE_MODE; - return U_NOT_IMPLEMENTED; + return U_UNKNOWN_REPLACE_MODE; } USTATUS FfsOperations::remove(const UModelIndex & index) diff --git a/common/ffsparser.cpp b/common/ffsparser.cpp index 754b97a..3170579 100644 --- a/common/ffsparser.cpp +++ b/common/ffsparser.cpp @@ -1399,6 +1399,7 @@ USTATUS FfsParser::parseFileHeader(const UByteArray & file, const UINT32 localOf ffsVersion = pdata->ffsVersion; volumeAlignment = pdata->alignment; volumeRevision = pdata->revision; + isWeakAligned = pdata->isWeakAligned; } // Get file header @@ -2478,7 +2479,7 @@ USTATUS FfsParser::parseGuidedSectionBody(const UModelIndex & index) } else { msg(usprintf("%s: can't guess the correct decompression algorithm, both preparse steps are failed", __FUNCTION__), index); - parseCurrentSection = false; + parseCurrentSection = false; } } @@ -2499,8 +2500,8 @@ USTATUS FfsParser::parseGuidedSectionBody(const UModelIndex & index) } else { info += UString("\nCompression algorithm: unknown"); - parseCurrentSection = false; - } + parseCurrentSection = false; + } } // Add info @@ -2933,13 +2934,13 @@ USTATUS FfsParser::addMemoryAddressesRecursive(const UModelIndex & index) // Determine relocation type of uncompressed TE image sections if (model->type(index) == Types::Section && model->subtype(index) == EFI_SECTION_TE) { // Obtain required values from parsing data - UINT32 imageBase = 0; + UINT32 originalImageBase = 0; UINT32 adjustedImageBase = 0; UINT8 imageBaseType = EFI_IMAGE_TE_BASE_OTHER; if (model->hasEmptyParsingData(index) == false) { UByteArray data = model->parsingData(index); const TE_IMAGE_SECTION_PARSING_DATA* pdata = (const TE_IMAGE_SECTION_PARSING_DATA*)data.constData(); - imageBase = pdata->imageBase; + originalImageBase = pdata->imageBase; adjustedImageBase = pdata->adjustedImageBase; } @@ -2974,7 +2975,7 @@ USTATUS FfsParser::addMemoryAddressesRecursive(const UModelIndex & index) // Update parsing data TE_IMAGE_SECTION_PARSING_DATA pdata; pdata.imageBaseType = imageBaseType; - pdata.imageBase = imageBase; + pdata.imageBase = originalImageBase; pdata.adjustedImageBase = adjustedImageBase; model->setParsingData(index, UByteArray((const char*)&pdata, sizeof(pdata))); } @@ -3246,8 +3247,8 @@ USTATUS FfsParser::parseVendorHashFile(const UByteArray & fileGuid, const UModel for (UINT32 i = 0; i < header->NumEntries; i++) { const BG_VENDOR_HASH_FILE_ENTRY* entry = (const BG_VENDOR_HASH_FILE_ENTRY*)(header + 1) + i; bootGuardInfo += usprintf("\nRelativeOffset: %08Xh Size: %Xh\nHash: ", entry->Offset, entry->Size); - for (UINT8 i = 0; i < sizeof(entry->Hash); i++) { - bootGuardInfo += usprintf("%02X", entry->Hash[i]); + for (UINT8 j = 0; j < sizeof(entry->Hash); j++) { + bootGuardInfo += usprintf("%02X", entry->Hash[j]); } } bootGuardInfo += UString("\n------------------------------------------------------------------------\n\n"); @@ -3285,8 +3286,8 @@ USTATUS FfsParser::parseVendorHashFile(const UByteArray & fileGuid, const UModel for (UINT32 i = 0; i < NumEntries; i++) { const BG_VENDOR_HASH_FILE_ENTRY* entry = (const BG_VENDOR_HASH_FILE_ENTRY*)(model->body(index).constData()) + i; bootGuardInfo += usprintf("\nAddress: %08Xh Size: %Xh\nHash: ", entry->Offset, entry->Size); - for (UINT8 i = 0; i < sizeof(entry->Hash); i++) { - bootGuardInfo += usprintf("%02X", entry->Hash[i]); + for (UINT8 j = 0; j < sizeof(entry->Hash); j++) { + bootGuardInfo += usprintf("%02X", entry->Hash[j]); } } bootGuardInfo += UString("\n------------------------------------------------------------------------\n\n"); @@ -3385,7 +3386,7 @@ USTATUS FfsParser::parseFit(const UModelIndex & index) currentStrings.push_back(usprintf("%04Xh", fitHeader->Version)); currentStrings.push_back(usprintf("%02Xh", fitHeader->Checksum)); currentStrings.push_back(fitEntryTypeToUString(fitHeader->Type)); - currentStrings.push_back(UString("")); // Empty info for FIT header + currentStrings.push_back(UString()); // Empty info for FIT header fitTable.push_back(std::pair, UModelIndex>(currentStrings, fitIndex)); // Process all other entries @@ -3844,13 +3845,13 @@ USTATUS FfsParser::parseIntelBootGuardBootPolicy(const UByteArray & bootPolicy, elementHeader->DataSize ); // Check for Microsoft PMDA hash data - const BG_MICROSOFT_PMDA_HEADER* header = (const BG_MICROSOFT_PMDA_HEADER*)(elementHeader + 1); - if (header->Version == BG_MICROSOFT_PMDA_VERSION - && elementHeader->DataSize == sizeof(BG_MICROSOFT_PMDA_HEADER) + sizeof(BG_MICROSOFT_PMDA_ENTRY)*header->NumEntries) { + const BG_MICROSOFT_PMDA_HEADER* pmdaHeader = (const BG_MICROSOFT_PMDA_HEADER*)(elementHeader + 1); + if (pmdaHeader->Version == BG_MICROSOFT_PMDA_VERSION + && elementHeader->DataSize == sizeof(BG_MICROSOFT_PMDA_HEADER) + sizeof(BG_MICROSOFT_PMDA_ENTRY)*pmdaHeader->NumEntries) { // Add entries bootGuardInfo += UString("\nMicrosoft PMDA-based protected ranges:\n"); - const BG_MICROSOFT_PMDA_ENTRY* entries = (const BG_MICROSOFT_PMDA_ENTRY*)(header + 1); - for (UINT32 i = 0; i < header->NumEntries; i++) { + const BG_MICROSOFT_PMDA_ENTRY* entries = (const BG_MICROSOFT_PMDA_ENTRY*)(pmdaHeader + 1); + for (UINT32 i = 0; i < pmdaHeader->NumEntries; i++) { bootGuardInfo += usprintf("Address: %08Xh Size: %08Xh\n", entries[i].Address, entries[i].Size); bootGuardInfo += UString("Hash: "); diff --git a/common/ffsparser.h b/common/ffsparser.h index ca17ab0..4274212 100644 --- a/common/ffsparser.h +++ b/common/ffsparser.h @@ -66,7 +66,7 @@ public: private: TreeModel *model; std::vector > messagesVector; - void msg(const UString message, const UModelIndex index = UModelIndex()) { + void msg(const UString & message, const UModelIndex & index = UModelIndex()) { messagesVector.push_back(std::pair(message, index)); }; diff --git a/common/nvramparser.cpp b/common/nvramparser.cpp index e7dcb7a..b253e13 100644 --- a/common/nvramparser.cpp +++ b/common/nvramparser.cpp @@ -937,18 +937,9 @@ USTATUS NvramParser::parseFdcStoreHeader(const UByteArray & store, const UINT32 return U_SUCCESS; } - // Check header size - UINT32 headerSize = sizeof(FDC_VOLUME_HEADER); - if (dataSize < headerSize) { - msg(usprintf("%s: FDC store header size %Xh (%u) is greater than volume body size %Xh (%u)", __FUNCTION__, - fdcStoreHeader->Size, fdcStoreHeader->Size, - dataSize, dataSize), parent); - return U_SUCCESS; - } - // Construct header and body - UByteArray header = store.left(headerSize); - UByteArray body = store.mid(headerSize, fdcStoreHeader->Size - headerSize); + UByteArray header = store.left(sizeof(FDC_VOLUME_HEADER)); + UByteArray body = store.mid(sizeof(FDC_VOLUME_HEADER), fdcStoreHeader->Size - sizeof(FDC_VOLUME_HEADER)); // Add info UString name("FDC store"); @@ -1391,7 +1382,7 @@ USTATUS NvramParser::parseVssStoreBody(const UModelIndex & index, UINT8 alignmen // Parse all variables while (1) { - bool isInvalid = false; + bool isInvalid = true; bool isAuthenticated = false; bool isAppleCrc32 = false; bool isIntelSpecial = false; @@ -1465,7 +1456,8 @@ USTATUS NvramParser::parseVssStoreBody(const UModelIndex & index, UINT8 alignmen } // Intel special variable - else if (variableHeader->State == NVRAM_VSS_INTEL_VARIABLE_VALID || variableHeader->State == NVRAM_VSS_INTEL_VARIABLE_INVALID) { + else if (variableHeader->State == NVRAM_VSS_INTEL_VARIABLE_VALID + || variableHeader->State == NVRAM_VSS_INTEL_VARIABLE_INVALID) { isIntelSpecial = true; const VSS_INTEL_VARIABLE_HEADER* intelVariableHeader = (const VSS_INTEL_VARIABLE_HEADER*)variableHeader; variableSize = intelVariableHeader->TotalSize; @@ -1483,7 +1475,7 @@ USTATUS NvramParser::parseVssStoreBody(const UModelIndex & index, UINT8 alignmen } // Normal VSS variable - if (!isAuthenticated && !isAppleCrc32 && !isIntelSpecial) { + else { variableSize = sizeof(VSS_VARIABLE_HEADER) + variableHeader->NameSize + variableHeader->DataSize; variableGuid = (EFI_GUID*)&variableHeader->VendorGuid; variableName = (CHAR16*)(variableHeader + 1); @@ -1493,8 +1485,10 @@ USTATUS NvramParser::parseVssStoreBody(const UModelIndex & index, UINT8 alignmen } // Check variable state - if (variableHeader->State != NVRAM_VSS_INTEL_VARIABLE_VALID && variableHeader->State != NVRAM_VSS_VARIABLE_ADDED && variableHeader->State != NVRAM_VSS_VARIABLE_HEADER_VALID) { - isInvalid = true; + if (variableHeader->State == NVRAM_VSS_INTEL_VARIABLE_VALID + || variableHeader->State == NVRAM_VSS_VARIABLE_ADDED + || variableHeader->State == NVRAM_VSS_VARIABLE_HEADER_VALID) { + isInvalid = false; } // Check variable size @@ -1565,8 +1559,9 @@ USTATUS NvramParser::parseVssStoreBody(const UModelIndex & index, UINT8 alignmen else if (isIntelSpecial) { subtype = Subtypes::IntelVssEntry; } - else + else { subtype = Subtypes::StandardVssEntry; + } // Add tree item model->addItem(localOffset + offset, Types::VssEntry, subtype, name, text, info, header, body, UByteArray(), Movable, index); @@ -1596,12 +1591,12 @@ USTATUS NvramParser::parseFsysStoreBody(const UModelIndex & index) const UByteArray data = model->body(index); // Check that the is enough space for variable header - const UINT32 dataSize = (UINT32)data.size(); + const UINT32 storeDataSize = (UINT32)data.size(); UINT32 offset = 0; // Parse all variables while (1) { - UINT32 unparsedSize = dataSize - offset; + UINT32 unparsedSize = storeDataSize - offset; UINT32 variableSize = 0; // Get nameSize and name of the variable @@ -1699,14 +1694,14 @@ USTATUS NvramParser::parseEvsaStoreBody(const UModelIndex & index) const UByteArray data = model->body(index); // Check that the is enough space for entry header - const UINT32 dataSize = (UINT32)data.size(); + const UINT32 storeDataSize = (UINT32)data.size(); UINT32 offset = 0; std::map guidMap; std::map nameMap; // Parse all entries - UINT32 unparsedSize = dataSize; + UINT32 unparsedSize = storeDataSize; while (unparsedSize) { UINT32 variableSize = 0; UString name; @@ -1721,8 +1716,8 @@ USTATUS NvramParser::parseEvsaStoreBody(const UModelIndex & index) // Check entry size variableSize = sizeof(EVSA_ENTRY_HEADER); if (unparsedSize < variableSize || unparsedSize < entryHeader->Size) { - UByteArray body = data.mid(offset); - UString info = usprintf("Full size: %Xh (%u)", body.size(), body.size()); + body = data.mid(offset); + info = usprintf("Full size: %Xh (%u)", body.size(), body.size()); if (body.count(emptyByte) == body.size()) { // Free space // Add free space tree item @@ -1813,8 +1808,8 @@ USTATUS NvramParser::parseEvsaStoreBody(const UModelIndex & index) } // Unknown entry or free space else { - UByteArray body = data.mid(offset); - UString info = usprintf("Full size: %Xh (%u)", body.size(), body.size()); + body = data.mid(offset); + info = usprintf("Full size: %Xh (%u)", body.size(), body.size()); if (body.count(emptyByte) == body.size()) { // Free space // Add free space tree item @@ -1835,7 +1830,7 @@ USTATUS NvramParser::parseEvsaStoreBody(const UModelIndex & index) // Move to next variable offset += variableSize; - unparsedSize = dataSize - offset; + unparsedSize = storeDataSize - offset; } // Reparse all data variables to detect invalid ones and assign name and test to valid ones diff --git a/common/nvramparser.h b/common/nvramparser.h index 5f99b86..facd4d2 100644 --- a/common/nvramparser.h +++ b/common/nvramparser.h @@ -44,7 +44,7 @@ private: TreeModel *model; FfsParser *ffsParser; std::vector > messagesVector; - void msg(const UString message, const UModelIndex index = UModelIndex()) { + void msg(const UString & message, const UModelIndex & index = UModelIndex()) { messagesVector.push_back(std::pair(message, index)); }; diff --git a/common/parsingdata.h b/common/parsingdata.h index b7d17b5..682770d 100644 --- a/common/parsingdata.h +++ b/common/parsingdata.h @@ -20,16 +20,16 @@ routines without the need of backward traversal #include "basetypes.h" typedef struct VOLUME_PARSING_DATA_ { - UINT8 ffsVersion; - UINT8 emptyByte; EFI_GUID extendedHeaderGuid; UINT32 alignment; + UINT32 usedSpace; + BOOLEAN hasValidUsedSpace; + UINT8 ffsVersion; + UINT8 emptyByte; UINT8 revision; BOOLEAN hasExtendedHeader; BOOLEAN hasAppleCrc32; BOOLEAN isWeakAligned; - BOOLEAN hasValidUsedSpace; - UINT32 usedSpace; } VOLUME_PARSING_DATA; typedef struct FILE_PARSING_DATA_ { diff --git a/common/peimage.h b/common/peimage.h index 91dea3e..a18aae9 100644 --- a/common/peimage.h +++ b/common/peimage.h @@ -586,8 +586,8 @@ typedef struct { } EFI_IMAGE_THUNK_DATA; #define EFI_IMAGE_ORDINAL_FLAG 0x80000000 // Flag for PE32. -#define EFI_IMAGE_SNAP_BY_ORDINAL(Ordinal) ((Ordinal & EFI_IMAGE_ORDINAL_FLAG) != 0) -#define EFI_IMAGE_ORDINAL(Ordinal) (Ordinal & 0xffff) +#define EFI_IMAGE_SNAP_BY_ORDINAL(Ordinal) (((Ordinal) & EFI_IMAGE_ORDINAL_FLAG) != 0) +#define EFI_IMAGE_ORDINAL(Ordinal) ((Ordinal) & 0xffff) // // Import Directory Table diff --git a/common/utility.cpp b/common/utility.cpp index c517e05..0cbccb0 100644 --- a/common/utility.cpp +++ b/common/utility.cpp @@ -186,12 +186,12 @@ UINT32 crc32(UINT32 initial, const UINT8* buffer, const UINT32 length) USTATUS decompress(const UByteArray & compressedData, const UINT8 compressionType, UINT8 & algorithm, UByteArray & decompressedData, UByteArray & efiDecompressedData) { const UINT8* data; - UINT32 dataSize; + UINT32 dataSize; UINT8* decompressed; UINT8* efiDecompressed; - UINT32 decompressedSize = 0; + UINT32 decompressedSize = 0; UINT8* scratch; - UINT32 scratchSize = 0; + UINT32 scratchSize = 0; const EFI_TIANO_HEADER* header; switch (compressionType) @@ -235,18 +235,25 @@ USTATUS decompress(const UByteArray & compressedData, const UINT8 compressionTyp // Try EFI 1.1 USTATUS EfiResult = EfiDecompress(data, dataSize, efiDecompressed, decompressedSize, scratch, scratchSize); + if (decompressedSize > INT32_MAX) { + free(decompressed); + free(efiDecompressed); + free(scratch); + return U_STANDARD_DECOMPRESSION_FAILED; + } + if (EfiResult == U_SUCCESS && TianoResult == U_SUCCESS) { // Both decompressions are OK algorithm = COMPRESSION_ALGORITHM_UNDECIDED; - decompressedData = UByteArray((const char*)decompressed, decompressedSize); - efiDecompressedData = UByteArray((const char*)efiDecompressed, decompressedSize); + decompressedData = UByteArray((const char*)decompressed, (int)decompressedSize); + efiDecompressedData = UByteArray((const char*)efiDecompressed, (int)decompressedSize); } else if (TianoResult == U_SUCCESS) { // Only Tiano is OK algorithm = COMPRESSION_ALGORITHM_TIANO; - decompressedData = UByteArray((const char*)decompressed, decompressedSize); + decompressedData = UByteArray((const char*)decompressed, (int)decompressedSize); } else if (EfiResult == U_SUCCESS) { // Only EFI 1.1 is OK algorithm = COMPRESSION_ALGORITHM_EFI11; - decompressedData = UByteArray((const char*)efiDecompressed, decompressedSize); + decompressedData = UByteArray((const char*)efiDecompressed, (int)decompressedSize); } else { // Both decompressions failed result = U_STANDARD_DECOMPRESSION_FAILED; @@ -293,13 +300,21 @@ USTATUS decompress(const UByteArray & compressedData, const UINT8 compressionTyp return U_CUSTOMIZED_DECOMPRESSION_FAILED; } else { + if (decompressedSize > INT32_MAX) { + free(decompressed); + return U_CUSTOMIZED_DECOMPRESSION_FAILED; + } algorithm = COMPRESSION_ALGORITHM_IMLZMA; - decompressedData = UByteArray((const char*)decompressed, decompressedSize); + decompressedData = UByteArray((const char*)decompressed, (int)decompressedSize); } } else { + if (decompressedSize > INT32_MAX) { + free(decompressed); + return U_CUSTOMIZED_DECOMPRESSION_FAILED; + } algorithm = COMPRESSION_ALGORITHM_LZMA; - decompressedData = UByteArray((const char*)decompressed, decompressedSize); + decompressedData = UByteArray((const char*)decompressed, (int)decompressedSize); } free(decompressed); @@ -330,7 +345,7 @@ UINT8 calculateChecksum8(const UINT8* buffer, UINT32 bufferSize) if (!buffer) return 0; - return (UINT8)0x100 - calculateSum8(buffer, bufferSize); + return (UINT8)(0x100U - calculateSum8(buffer, bufferSize)); } // 16bit checksum calculation routine