From aa3e790fb1c7a18e4661c5c5111f13c701d8ba62 Mon Sep 17 00:00:00 2001 From: Nikolaj Schlej Date: Sun, 12 Mar 2023 08:49:19 -0700 Subject: [PATCH] Revert "Fix FfsParser issues found by fuzzing" This reverts commit b8567d32cc158eb68d900d9a161e92889e643627. --- UEFIExtract/ffsdumper.cpp | 6 +-- UEFIFind/uefifind.cpp | 4 +- common/ffs.cpp | 4 +- common/ffs.h | 2 +- common/ffsparser.cpp | 97 ++++++++++++++++-------------------- common/guiddatabase.cpp | 2 +- common/nvramparser.cpp | 27 +++++----- fuzzing/ffsparser_fuzzer.cpp | 3 -- 8 files changed, 64 insertions(+), 81 deletions(-) diff --git a/UEFIExtract/ffsdumper.cpp b/UEFIExtract/ffsdumper.cpp index 7834ee3..cb4b617 100644 --- a/UEFIExtract/ffsdumper.cpp +++ b/UEFIExtract/ffsdumper.cpp @@ -48,9 +48,9 @@ USTATUS FfsDumper::recursiveDump(const UModelIndex & index, const UString & path if (guid.isEmpty() || (model->subtype(index) == EFI_SECTION_FREEFORM_SUBTYPE_GUID && - guidToUString(model->header(index).constData() + sizeof(EFI_COMMON_SECTION_HEADER)) == guid) || - guidToUString(model->header(index).constData()) == guid || - guidToUString(model->header(model->findParentOfType(index, Types::File)).constData()) == guid) { + guidToUString(readUnaligned((const EFI_GUID*)(model->header(index).constData() + sizeof(EFI_COMMON_SECTION_HEADER)))) == guid) || + guidToUString(readUnaligned((const EFI_GUID*)model->header(index).constData())) == guid || + guidToUString(readUnaligned((const EFI_GUID*)model->header(model->findParentOfType(index, Types::File)).constData())) == guid) { if (!changeDirectory(path) && !makeDirectory(path)) { printf("Cannot use directory \"%s\" (recursiveDump part 1).\n", (const char*)path.toLocal8Bit()); diff --git a/UEFIFind/uefifind.cpp b/UEFIFind/uefifind.cpp index 1b98670..4e7412a 100644 --- a/UEFIFind/uefifind.cpp +++ b/UEFIFind/uefifind.cpp @@ -136,12 +136,12 @@ USTATUS UEFIFind::find(const UINT8 mode, const bool count, const UString & hexPa std::pair indexes = *citer; if (!model->hasEmptyHeader(indexes.first)) data = model->header(indexes.first).left(16); - result += guidToUString(data.constData()); + result += guidToUString(readUnaligned((const EFI_GUID*)data.constData())); // Special case of freeform subtype GUID files if (indexes.second.isValid() && model->subtype(indexes.second) == EFI_SECTION_FREEFORM_SUBTYPE_GUID) { data = model->header(indexes.second); - result += UString(" ") + (guidToUString(data.constData() + sizeof(EFI_COMMON_SECTION_HEADER))); + result += UString(" ") + (guidToUString(readUnaligned((const EFI_GUID*)(data.constData() + sizeof(EFI_COMMON_SECTION_HEADER))))); } result += UString("\n"); diff --git a/common/ffs.cpp b/common/ffs.cpp index 9811ab4..3025df1 100644 --- a/common/ffs.cpp +++ b/common/ffs.cpp @@ -178,10 +178,8 @@ UINT32 uint24ToUint32(const UINT8* ffsSize) + ((UINT32) ffsSize[2] << 16U); } -UString guidToUString(const char* in, bool convertToString) +UString guidToUString(const EFI_GUID & guid, bool convertToString) { - const EFI_GUID guid = readUnaligned((EFI_GUID*)in); - if (convertToString) { UString readableName = guidDatabaseLookup(guid); if (!readableName.isEmpty()) diff --git a/common/ffs.h b/common/ffs.h index d40c89f..eeb8c44 100644 --- a/common/ffs.h +++ b/common/ffs.h @@ -22,7 +22,7 @@ WITHWARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. // Make sure we use right packing rules #pragma pack(push,1) -extern UString guidToUString(const char* in, bool convertToString = true); +extern UString guidToUString(const EFI_GUID& guid, bool convertToString = true); extern bool ustringToGuid(const UString& str, EFI_GUID& guid); extern UString fileTypeToUString(const UINT8 type); extern UString sectionTypeToUString(const UINT8 type); diff --git a/common/ffsparser.cpp b/common/ffsparser.cpp index b5220fd..6701926 100644 --- a/common/ffsparser.cpp +++ b/common/ffsparser.cpp @@ -175,7 +175,7 @@ USTATUS FfsParser::parseCapsule(const UByteArray & capsule, const UINT32 localOf UByteArray header = capsule.left(capsuleHeaderSize); UByteArray body = capsule.mid(capsuleHeaderSize); UString name("UEFI capsule"); - UString info = UString("Capsule GUID: ") + guidToUString((const char*)&capsuleHeader->CapsuleGuid, false) + + UString info = UString("Capsule GUID: ") + guidToUString(capsuleHeader->CapsuleGuid, false) + usprintf("\nFull size: %Xh (%u)\nHeader size: %Xh (%u)\nImage size: %Xh (%u)\nFlags: %08Xh", (UINT32)capsule.size(), (UINT32)capsule.size(), capsuleHeaderSize, capsuleHeaderSize, @@ -207,7 +207,7 @@ USTATUS FfsParser::parseCapsule(const UByteArray & capsule, const UINT32 localOf UByteArray header = capsule.left(capsuleHeaderSize); UByteArray body = capsule.mid(capsuleHeaderSize); UString name("Toshiba capsule"); - UString info = UString("Capsule GUID: ") + guidToUString((const char*)&capsuleHeader->CapsuleGuid, false) + + UString info = UString("Capsule GUID: ") + guidToUString(capsuleHeader->CapsuleGuid, false) + usprintf("\nFull size: %Xh (%u)\nHeader size: %Xh (%u)\nImage size: %Xh (%u)\nFlags: %08Xh", (UINT32)capsule.size(), (UINT32)capsule.size(), capsuleHeaderSize, capsuleHeaderSize, @@ -248,7 +248,7 @@ USTATUS FfsParser::parseCapsule(const UByteArray & capsule, const UINT32 localOf UByteArray header = capsule.left(capsuleHeaderSize); UByteArray body = capsule.mid(capsuleHeaderSize); UString name("AMI Aptio capsule"); - UString info = UString("Capsule GUID: ") + guidToUString((const char*)&capsuleHeader->CapsuleHeader.CapsuleGuid, false) + + UString info = UString("Capsule GUID: ") + guidToUString(capsuleHeader->CapsuleHeader.CapsuleGuid, false) + usprintf("\nFull size: %Xh (%u)\nHeader size: %Xh (%u)\nImage size: %Xh (%u)\nFlags: %08Xh", (UINT32)capsule.size(), (UINT32)capsule.size(), capsuleHeaderSize, capsuleHeaderSize, @@ -1011,6 +1011,7 @@ USTATUS FfsParser::parseVolumeHeader(const UByteArray & volume, const UINT32 loc // Check that there is space for the volume header if ((UINT32)volume.size() < sizeof(EFI_FIRMWARE_VOLUME_HEADER)) { + msg(usprintf("%s: input volume size %Xh (%u) is smaller than volume header size 40h (64)", __FUNCTION__, (UINT32)volume.size(), (UINT32)volume.size())); return U_INVALID_VOLUME; } @@ -1019,11 +1020,13 @@ USTATUS FfsParser::parseVolumeHeader(const UByteArray & volume, const UINT32 loc // Check sanity of HeaderLength value if ((UINT32)ALIGN8(volumeHeader->HeaderLength) > (UINT32)volume.size()) { + msg(usprintf("%s: volume header overlaps the end of data", __FUNCTION__)); return U_INVALID_VOLUME; } // Check sanity of ExtHeaderOffset value if (volumeHeader->Revision > 1 && volumeHeader->ExtHeaderOffset && (UINT32)ALIGN8(volumeHeader->ExtHeaderOffset + sizeof(EFI_FIRMWARE_VOLUME_EXT_HEADER)) > (UINT32)volume.size()) { + msg(usprintf("%s: extended volume header overlaps the end of data", __FUNCTION__)); return U_INVALID_VOLUME; } @@ -1073,11 +1076,6 @@ USTATUS FfsParser::parseVolumeHeader(const UByteArray & volume, const UINT32 loc headerSize = EFI_APPLE_MICROCODE_VOLUME_HEADER_SIZE; } - // Check calculated size, must leave at least 1 byte for the body - if (headerSize >= (UINT32)volume.size()) { - return U_INVALID_VOLUME; - } - // Check volume revision and alignment bool msgAlignmentBitsSet = false; bool msgUnaligned = false; @@ -1113,8 +1111,8 @@ USTATUS FfsParser::parseVolumeHeader(const UByteArray & volume, const UINT32 loc // Check for AppleCRC32 and UsedSpace in ZeroVector bool hasAppleCrc32 = false; UINT32 volumeSize = (UINT32)volume.size(); - UINT32 appleCrc32 = readUnaligned((UINT32*)(volume.constData() + 8)); - UINT32 usedSpace = readUnaligned((UINT32*)(volume.constData() + 12)); + UINT32 appleCrc32 = *(UINT32*)(volume.constData() + 8); + UINT32 usedSpace = *(UINT32*)(volume.constData() + 12); if (appleCrc32 != 0) { // Calculate CRC32 of the volume body UINT32 crc = (UINT32)crc32(0, (const UINT8*)(volume.constData() + volumeHeader->HeaderLength), volumeSize - volumeHeader->HeaderLength); @@ -1134,18 +1132,18 @@ USTATUS FfsParser::parseVolumeHeader(const UByteArray & volume, const UINT32 loc // Get info UByteArray header = volume.left(headerSize); UByteArray body = volume.mid(headerSize); - UString name = guidToUString((const char*)&volumeHeader->FileSystemGuid); + UString name = guidToUString(volumeHeader->FileSystemGuid); UString info = usprintf("ZeroVector:\n%02X %02X %02X %02X %02X %02X %02X %02X\n" "%02X %02X %02X %02X %02X %02X %02X %02X\nSignature: _FVH\nFileSystem GUID: ", volumeHeader->ZeroVector[0], volumeHeader->ZeroVector[1], volumeHeader->ZeroVector[2], volumeHeader->ZeroVector[3], volumeHeader->ZeroVector[4], volumeHeader->ZeroVector[5], volumeHeader->ZeroVector[6], volumeHeader->ZeroVector[7], volumeHeader->ZeroVector[8], volumeHeader->ZeroVector[9], volumeHeader->ZeroVector[10], volumeHeader->ZeroVector[11], volumeHeader->ZeroVector[12], volumeHeader->ZeroVector[13], volumeHeader->ZeroVector[14], volumeHeader->ZeroVector[15]) - + guidToUString((const char*)&volumeHeader->FileSystemGuid, false) \ + + guidToUString(volumeHeader->FileSystemGuid, false) \ + usprintf("\nFull size: %Xh (%u)\nHeader size: %Xh (%u)\nBody size: %Xh (%u)\nRevision: %u\nAttributes: %08Xh\nErase polarity: %u\nChecksum: %04Xh", volumeSize, volumeSize, - (UINT32)header.size(), (UINT32)header.size(), - (UINT32)body.size(), (UINT32)body.size(), + headerSize, headerSize, + volumeSize - headerSize, volumeSize - headerSize, volumeHeader->Revision, volumeHeader->Attributes, (emptyByte ? 1 : 0), @@ -1156,8 +1154,8 @@ USTATUS FfsParser::parseVolumeHeader(const UByteArray & volume, const UINT32 loc if (volumeHeader->Revision > 1 && volumeHeader->ExtHeaderOffset) { const EFI_FIRMWARE_VOLUME_EXT_HEADER* extendedHeader = (const EFI_FIRMWARE_VOLUME_EXT_HEADER*)(volume.constData() + volumeHeader->ExtHeaderOffset); info += usprintf("\nExtended header size: %Xh (%u)\nVolume GUID: ", - extendedHeader->ExtHeaderSize, extendedHeader->ExtHeaderSize) + guidToUString((const char*)&extendedHeader->FvName, false); - name = guidToUString((const char*)&extendedHeader->FvName); // Replace FFS GUID with volume GUID + extendedHeader->ExtHeaderSize, extendedHeader->ExtHeaderSize) + guidToUString(extendedHeader->FvName, false); + name = guidToUString(extendedHeader->FvName); // Replace FFS GUID with volume GUID } // Add text @@ -1195,7 +1193,7 @@ USTATUS FfsParser::parseVolumeHeader(const UByteArray & volume, const UINT32 loc // Show messages if (isUnknown) - msg(usprintf("%s: unknown file system ", __FUNCTION__) + guidToUString((const char*)&volumeHeader->FileSystemGuid), index); + msg(usprintf("%s: unknown file system ", __FUNCTION__) + guidToUString(volumeHeader->FileSystemGuid), index); if (msgInvalidChecksum) msg(usprintf("%s: volume header checksum is invalid", __FUNCTION__), index); if (msgAlignmentBitsSet) @@ -1274,9 +1272,6 @@ bool FfsParser::microcodeHeaderValid(const INTEL_MICROCODE_HEADER* ucodeHeader) USTATUS FfsParser::findNextRawAreaItem(const UModelIndex & index, const UINT32 localOffset, UINT8 & nextItemType, UINT32 & nextItemOffset, UINT32 & nextItemSize, UINT32 & nextItemAlternativeSize) { - if (!index.isValid()) - return U_INVALID_PARAMETER; - UByteArray data = model->body(index); UINT32 dataSize = (UINT32)data.size(); @@ -1311,16 +1306,11 @@ USTATUS FfsParser::findNextRawAreaItem(const UModelIndex & index, const UINT32 l break; } else if (readUnaligned(currentPos) == EFI_FV_SIGNATURE) { - if (restSize < sizeof(EFI_FIRMWARE_VOLUME_HEADER)) + if (offset < EFI_FV_SIGNATURE_OFFSET) continue; - - if (offset < EFI_FV_SIGNATURE_OFFSET + sizeof(const EFI_FIRMWARE_VOLUME_HEADER)) - continue; - - UINT32 localOffset = offset - EFI_FV_SIGNATURE_OFFSET; - const EFI_FIRMWARE_VOLUME_HEADER* volumeHeader = (const EFI_FIRMWARE_VOLUME_HEADER*)(data.constData() + localOffset); - if (volumeHeader->FvLength < sizeof(EFI_FIRMWARE_VOLUME_HEADER) + 2 * sizeof(EFI_FV_BLOCK_MAP_ENTRY) - || volumeHeader->FvLength >= 0xFFFFFFFFUL) { + + const EFI_FIRMWARE_VOLUME_HEADER* volumeHeader = (const EFI_FIRMWARE_VOLUME_HEADER*)(data.constData() + offset - EFI_FV_SIGNATURE_OFFSET); + if (volumeHeader->FvLength < sizeof(EFI_FIRMWARE_VOLUME_HEADER) + 2 * sizeof(EFI_FV_BLOCK_MAP_ENTRY) || volumeHeader->FvLength >= 0xFFFFFFFFUL) { continue; } if (volumeHeader->Revision != 1 && volumeHeader->Revision != 2) { @@ -1328,18 +1318,15 @@ USTATUS FfsParser::findNextRawAreaItem(const UModelIndex & index, const UINT32 l } // Calculate alternative volume size using its BlockMap - if (localOffset < sizeof(EFI_FIRMWARE_VOLUME_HEADER) + 2 * sizeof(EFI_FV_BLOCK_MAP_ENTRY)) - continue; - nextItemAlternativeSize = 0; - const EFI_FV_BLOCK_MAP_ENTRY* entry = (const EFI_FV_BLOCK_MAP_ENTRY*)(data.constData() + localOffset + sizeof(EFI_FIRMWARE_VOLUME_HEADER)); + const EFI_FV_BLOCK_MAP_ENTRY* entry = (const EFI_FV_BLOCK_MAP_ENTRY*)(data.constData() + offset - EFI_FV_SIGNATURE_OFFSET + sizeof(EFI_FIRMWARE_VOLUME_HEADER)); while (entry->NumBlocks != 0 && entry->Length != 0) { - // Check if we are getting past the end of the volume - if ((const void*)entry >= data.constData() + dataSize) { + // Check if we are past the end of the volume + if ((const void*)entry >= data.constData() + data.size()) { // This volume is broken, but we can't use continue here because we need to continue the outer loop goto continue_searching; } - + nextItemAlternativeSize += entry->NumBlocks * entry->Length; entry += 1; } @@ -1347,8 +1334,9 @@ USTATUS FfsParser::findNextRawAreaItem(const UModelIndex & index, const UINT32 l // All checks passed, volume found nextItemType = Types::Volume; nextItemSize = (UINT32)volumeHeader->FvLength; - nextItemOffset = localOffset; + nextItemOffset = offset - EFI_FV_SIGNATURE_OFFSET; break; +continue_searching: {} } else if (readUnaligned(currentPos) == BPDT_GREEN_SIGNATURE || readUnaligned(currentPos) == BPDT_YELLOW_SIGNATURE) { @@ -1402,7 +1390,6 @@ USTATUS FfsParser::findNextRawAreaItem(const UModelIndex & index, const UINT32 l nextItemOffset = offset; break; } -continue_searching: {} } // No more stores found @@ -1591,7 +1578,7 @@ USTATUS FfsParser::parseVolumeBody(const UModelIndex & index) // Check GUIDs for being equal if (currentGuid == anotherGuid) { - msg(usprintf("%s: file with duplicate GUID ", __FUNCTION__) + guidToUString(anotherGuid.constData()), another); + msg(usprintf("%s: file with duplicate GUID ", __FUNCTION__) + guidToUString(readUnaligned((EFI_GUID*)(anotherGuid.data()))), another); } } } @@ -1720,9 +1707,9 @@ USTATUS FfsParser::parseFileHeader(const UByteArray & file, const UINT32 localOf // Check for file tail presence UByteArray tail; bool msgInvalidTailValue = false; - if (volumeRevision == 1 && (fileHeader->Attributes & FFS_ATTRIB_TAIL_PRESENT) && body.size() > sizeof(UINT16)) { + if (volumeRevision == 1 && (fileHeader->Attributes & FFS_ATTRIB_TAIL_PRESENT)) { //Check file tail; - UINT16 tailValue = readUnaligned((UINT16*)body.right(sizeof(UINT16)).constData()); + UINT16 tailValue = *(UINT16*)body.right(sizeof(UINT16)).constData(); if (fileHeader->IntegrityCheck.TailReference != (UINT16)~tailValue) msgInvalidTailValue = true; @@ -1767,12 +1754,12 @@ USTATUS FfsParser::parseFileHeader(const UByteArray & file, const UINT32 localOf UString name; UString info; if (fileHeader->Type != EFI_FV_FILETYPE_PAD) { - name = guidToUString((const char*)&fileHeader->Name); + name = guidToUString(fileHeader->Name); } else { name = UString("Padding file"); } - info = UString("File GUID: ") + guidToUString((const char*)&fileHeader->Name, false) + + info = UString("File GUID: ") + guidToUString(fileHeader->Name, false) + usprintf("\nType: %02Xh\nAttributes: %02Xh\nFull size: %Xh (%u)\nHeader size: %Xh (%u)\nBody size: %Xh (%u)\nTail size: %Xh (%u)\nState: %02Xh", fileHeader->Type, fileHeader->Attributes, @@ -1835,7 +1822,7 @@ USTATUS FfsParser::parseFileHeader(const UByteArray & file, const UINT32 localOf if (msgInvalidDataChecksum) msg(usprintf("%s: invalid data checksum %02Xh, should be %02Xh", __FUNCTION__, fileHeader->IntegrityCheck.Checksum.File, calculatedData), index); if (msgInvalidTailValue) - msg(usprintf("%s: invalid tail value %04Xh", __FUNCTION__, readUnaligned((UINT16*)tail.constData())), index); + msg(usprintf("%s: invalid tail value %04Xh", __FUNCTION__, *(const UINT16*)tail.constData()), index); if (msgUnknownType) msg(usprintf("%s: unknown file type %02Xh", __FUNCTION__, fileHeader->Type), index); @@ -2345,7 +2332,7 @@ USTATUS FfsParser::parseGuidedSectionHeader(const UByteArray & section, const UI if ((UINT32)section.size() < headerSize + sizeof(UINT32)) return U_INVALID_SECTION; - UINT32 crc = readUnaligned((UINT32*)(section.constData() + headerSize)); + UINT32 crc = *(UINT32*)(section.constData() + headerSize); additionalInfo += UString("\nChecksum type: CRC32"); // Calculate CRC32 of section data UINT32 calculated = (UINT32)crc32(0, (const UINT8*)section.constData() + dataOffset, (uInt)(section.size() - dataOffset)); @@ -2432,7 +2419,7 @@ USTATUS FfsParser::parseGuidedSectionHeader(const UByteArray & section, const UI additionalInfo += UString("\nCertificate subtype: RSA2048/SHA256"); } else { - additionalInfo += UString("\nCertificate subtype: unknown, GUID ") + guidToUString((const char*)&winCertificateUefiGuid->CertType); + additionalInfo += UString("\nCertificate subtype: unknown, GUID ") + guidToUString(winCertificateUefiGuid->CertType); msgUnknownCertSubtype = true; } } @@ -2451,8 +2438,8 @@ USTATUS FfsParser::parseGuidedSectionHeader(const UByteArray & section, const UI UByteArray body = section.mid(dataOffset); // Get info - UString name = guidToUString((const char*)&guid); - UString info = UString("Section GUID: ") + guidToUString((const char*)&guid, false) + + UString name = guidToUString(guid); + UString info = UString("Section GUID: ") + guidToUString(guid, false) + usprintf("\nType: %02Xh\nFull size: %Xh (%u)\nHeader size: %Xh (%u)\nBody size: %Xh (%u)\nAttributes: %04Xh", sectionHeader->Type, (UINT32)section.size(), (UINT32)section.size(), @@ -2547,7 +2534,7 @@ USTATUS FfsParser::parseFreeformGuidedSectionHeader(const UByteArray & section, (UINT32)section.size(), (UINT32)section.size(), (UINT32)header.size(), (UINT32)header.size(), (UINT32)body.size(), (UINT32)body.size()) - + guidToUString((const char*)&guid, false); + + guidToUString(guid, false); // Add tree item if (insertIntoTree) { @@ -2559,7 +2546,7 @@ USTATUS FfsParser::parseFreeformGuidedSectionHeader(const UByteArray & section, model->setParsingData(index, UByteArray((const char*)&pdata, sizeof(pdata))); // Rename section - model->setName(index, guidToUString((const char*)&guid)); + model->setName(index, guidToUString(guid)); } return U_SUCCESS; @@ -2969,7 +2956,7 @@ USTATUS FfsParser::parseDepexSectionBody(const UModelIndex & index) return U_SUCCESS; } guid = (const EFI_GUID*)(current + EFI_DEP_OPCODE_SIZE); - parsed += UString("\nBEFORE ") + guidToUString((const char*)guid); + parsed += UString("\nBEFORE ") + guidToUString(readUnaligned(guid)); current += EFI_DEP_OPCODE_SIZE + sizeof(EFI_GUID); if (*current != EFI_DEP_END){ msg(usprintf("%s: DEPEX section ends with non-END opcode", __FUNCTION__), index); @@ -2983,7 +2970,7 @@ USTATUS FfsParser::parseDepexSectionBody(const UModelIndex & index) return U_SUCCESS; } guid = (const EFI_GUID*)(current + EFI_DEP_OPCODE_SIZE); - parsed += UString("\nAFTER ") + guidToUString((const char*)guid); + parsed += UString("\nAFTER ") + guidToUString(readUnaligned(guid)); current += EFI_DEP_OPCODE_SIZE + sizeof(EFI_GUID); if (*current != EFI_DEP_END) { msg(usprintf("%s: DEPEX section ends with non-END opcode", __FUNCTION__), index); @@ -3024,7 +3011,7 @@ USTATUS FfsParser::parseDepexSectionBody(const UModelIndex & index) return U_SUCCESS; } guid = (const EFI_GUID*)(current + EFI_DEP_OPCODE_SIZE); - parsed += UString("\nPUSH ") + guidToUString((const char*)guid); + parsed += UString("\nPUSH ") + guidToUString(readUnaligned(guid)); current += EFI_DEP_OPCODE_SIZE + sizeof(EFI_GUID); break; case EFI_DEP_AND: @@ -3097,7 +3084,7 @@ USTATUS FfsParser::parseAprioriRawSection(const UByteArray & body, UString & par if (count > 0) { for (UINT32 i = 0; i < count; i++) { const EFI_GUID* guid = (const EFI_GUID*)body.constData() + i; - parsed += "\n" + guidToUString((const char*)guid); + parsed += "\n" + guidToUString(readUnaligned(guid)); } } @@ -4222,7 +4209,6 @@ USTATUS FfsParser::parseBpdtRegion(const UByteArray & region, const UINT32 local } } -make_partition_table_consistent: // Check for empty set of partitions if (partitions.empty()) { // Add a single padding partition in this case @@ -4233,6 +4219,7 @@ make_partition_table_consistent: partitions.push_back(padding); } +make_partition_table_consistent: // Sort partitions by offset std::sort(partitions.begin(), partitions.end()); diff --git a/common/guiddatabase.cpp b/common/guiddatabase.cpp index 42c3c5c..c8aa89d 100644 --- a/common/guiddatabase.cpp +++ b/common/guiddatabase.cpp @@ -133,7 +133,7 @@ USTATUS guidDatabaseExportToFile(const UString & outPath, GuidDatabase & db) if (!outputFile) return U_FILE_OPEN; for (GuidDatabase::iterator it = db.begin(); it != db.end(); it++) { - std::string guid(guidToUString((const char*)&it->first, false).toLocal8Bit()); + std::string guid(guidToUString (it->first, false).toLocal8Bit()); std::string name(it->second.toLocal8Bit()); outputFile << guid << ',' << name << '\n'; } diff --git a/common/nvramparser.cpp b/common/nvramparser.cpp index 3b7b682..68b7454 100755 --- a/common/nvramparser.cpp +++ b/common/nvramparser.cpp @@ -172,8 +172,9 @@ USTATUS NvramParser::parseNvarStore(const UModelIndex & index) // Obtain GUID if (!entry_body->_is_null_guid()) { // GUID is stored in the entry itself - name = guidToUString(entry_body->guid().c_str()); - guid = guidToUString(entry_body->guid().c_str(), false); + const EFI_GUID g = readUnaligned((EFI_GUID*)entry_body->guid().c_str()); + name = guidToUString(g); + guid = guidToUString(g, false); } else { // GUID is stored in GUID store at the end of the NVAR store // Grow the GUID store if needed @@ -181,7 +182,7 @@ USTATUS NvramParser::parseNvarStore(const UModelIndex & index) guidsInStore = entry_body->guid_index() + 1; // The list begins at the end of the store and goes backwards - const char *g = (nvar.constData() + nvar.size()) - (entry_body->guid_index() + 1); + const EFI_GUID g = readUnaligned((EFI_GUID*)(nvar.constData() + nvar.size()) - (entry_body->guid_index() + 1)); name = guidToUString(g); guid = guidToUString(g, false); } @@ -764,7 +765,7 @@ USTATUS NvramParser::parseVss2StoreHeader(const UByteArray & store, const UINT32 // Add info UString name = UString("VSS2 store"); - UString info = UString("Signature: ") + guidToUString((const char*)&vssStoreHeader->Signature, false) + + UString info = UString("Signature: ") + guidToUString(vssStoreHeader->Signature, false) + usprintf("\nFull size: %Xh (%u)\nHeader size: %Xh (%u)\nBody size: %Xh (%u)\nFormat: %02Xh\nState: %02Xh\nUnknown: %04Xh", storeSize, storeSize, (UINT32)header.size(), (UINT32)header.size(), @@ -834,7 +835,7 @@ USTATUS NvramParser::parseFtwStoreHeader(const UByteArray & store, const UINT32 // Add info UString name("FTW store"); - UString info = UString("Signature: ") + guidToUString((const char*)&ftw32BlockHeader->Signature, false) + + UString info = UString("Signature: ") + guidToUString(ftw32BlockHeader->Signature, false) + usprintf("\nFull size: %Xh (%u)\nHeader size: %Xh (%u)\nBody size: %Xh (%u)\nState: %02Xh\nHeader CRC32: %08Xh", ftwBlockSize, ftwBlockSize, headerSize, headerSize, @@ -1415,8 +1416,8 @@ USTATUS NvramParser::parseVssStoreBody(const UModelIndex & index, UINT8 alignmen name = UString("Invalid"); } else { // Add GUID and text for valid variables - name = guidToUString((const char*)variableGuid); - info += UString("Variable GUID: ") + guidToUString((const char*)variableGuid, false) + "\n"; + name = guidToUString(readUnaligned(variableGuid)); + info += UString("Variable GUID: ") + guidToUString(readUnaligned(variableGuid), false) + "\n"; text = uFromUcs2((const char*)variableName); } @@ -1629,9 +1630,9 @@ USTATUS NvramParser::parseEvsaStoreBody(const UModelIndex & index) const EVSA_GUID_ENTRY* guidHeader = (const EVSA_GUID_ENTRY*)entryHeader; header = data.mid(offset, sizeof(EVSA_GUID_ENTRY)); body = data.mid(offset + sizeof(EVSA_GUID_ENTRY), guidHeader->Header.Size - sizeof(EVSA_GUID_ENTRY)); - EFI_GUID guid = readUnaligned((EFI_GUID*)body.constData()); - name = guidToUString(body.constData()); - info = UString("GUID: ") + guidToUString(body.constData(), false) + EFI_GUID guid = *(EFI_GUID*)body.constData(); + name = guidToUString(guid); + info = UString("GUID: ") + guidToUString(guid, false) + usprintf("\nFull size: %Xh (%u)\nHeader size: %Xh (%u)\nBody size: %Xh (%u)\nType: %02Xh\nChecksum: %02Xh", variableSize, variableSize, (UINT32)header.size(), (UINT32)header.size(), @@ -1730,7 +1731,7 @@ USTATUS NvramParser::parseEvsaStoreBody(const UModelIndex & index) const EVSA_DATA_ENTRY* dataHeader = (const EVSA_DATA_ENTRY*)header.constData(); UString guid; if (guidMap.count(dataHeader->GuidId)) - guid = guidToUString((const char*)&guidMap[dataHeader->GuidId], false); + guid = guidToUString(guidMap[dataHeader->GuidId], false); UString name; if (nameMap.count(dataHeader->VarId)) name = nameMap[dataHeader->VarId]; @@ -1803,13 +1804,13 @@ USTATUS NvramParser::parseFlashMapBody(const UModelIndex & index) break; } - UString name = guidToUString((const char*)&entryHeader->Guid); + UString name = guidToUString(entryHeader->Guid); // Construct header UByteArray header = data.mid(offset, sizeof(PHOENIX_FLASH_MAP_ENTRY)); // Add info - UString info = UString("Entry GUID: ") + guidToUString((const char*)&entryHeader->Guid, false) + + UString info = UString("Entry GUID: ") + guidToUString(entryHeader->Guid, false) + usprintf("\nFull size: 24h (36)\nHeader size: 24h (36)\nBody size: 0h (0)\n" "Entry type: %04Xh\nData type: %04Xh\nMemory address: %08Xh\nSize: %08Xh\nOffset: %08Xh", entryHeader->EntryType, diff --git a/fuzzing/ffsparser_fuzzer.cpp b/fuzzing/ffsparser_fuzzer.cpp index 47d722b..2411952 100644 --- a/fuzzing/ffsparser_fuzzer.cpp +++ b/fuzzing/ffsparser_fuzzer.cpp @@ -27,8 +27,5 @@ extern "C" int LLVMFuzzerTestOneInput(const char *Data, long long Size) { // Parse the image (void)ffsParser->parse(UByteArray(Data, (uint32_t)Size)); - delete ffsParser; - delete model; - return 0; }