Merge pull request #1120 from avast/image-loader-page-size-sanity-check

Added sanity check for page index when loading pages from broken samples
avast · Nov 7, 2022 · 0c505b4 · 0c505b4
2 parents 74c28cd + 6aa3732
commit 0c505b4
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 6 deletions.
diff --git a/include/retdec/pelib/ImageLoader.h b/include/retdec/pelib/ImageLoader.h
@@ -53,7 +53,7 @@ enum struct PELIB_COMPARE_RESULT : std::uint32_t
 //-----------------------------------------------------------------------------
 // Windows build numbers
 
-const std::uint32_t	BuildNumberXP = 2600;           // Behavior equal to Windows XP
+const std::uint32_t BuildNumberXP = 2600;           // Behavior equal to Windows XP
 const std::uint32_t BuildNumberVista = 6000;        // Behavior equal to Windows Vista (SP0 = 6000, SP1 = 6001, SP2 = 6002)
 const std::uint32_t BuildNumber7 = 7600;            // Behavior equal to Windows 7 (SP0 = 7600, SP1 = 7601)
 const std::uint32_t BuildNumber8 = 9200;            // Behavior equal to Windows 8

diff --git a/src/pelib/ImageLoader.cpp b/src/pelib/ImageLoader.cpp
@@ -2239,15 +2239,21 @@ int PeLib::ImageLoader::captureImageSections(ByteBuffer & fileData, std::uint32_
 			{
 				// If loading as image, we need to take data from its virtual address
 				std::uint32_t pointerToRawData = (loadFlags & IoFlagLoadAsImage) ? sectionHeader.VirtualAddress : sectionHeader.PointerToRawData;
+				std::uint32_t sectionEnd;
 
 				// Capture all pages from the section
-				if(captureImageSection(fileData, sectionHeader.VirtualAddress,
+				sectionEnd = captureImageSection(fileData,
+												 sectionHeader.VirtualAddress,
 												 sectionHeader.VirtualSize,
 												 pointerToRawData,
 												 sectionHeader.SizeOfRawData,
-												 sectionHeader.Characteristics) == 0)
+												 sectionHeader.Characteristics);
+
+				// There must not be a Virtual Address overflow,
+				// nor the end of the section must be beyond the end of the image
+				if(sectionEnd < sectionHeader.VirtualAddress || sectionEnd > sizeOfImage)
 				{
-					setLoaderError(LDR_ERROR_INVALID_SECTION_VA);
+					setLoaderError(LDR_ERROR_INVALID_SECTION_VSIZE);
 					break;
 				}
 			}
@@ -2522,7 +2528,13 @@ std::uint32_t PeLib::ImageLoader::captureImageSection(
 	// * has 0x1000 bytes of inaccessible memory at ImageBase+0x1000 (1 page after section header)
 	sizeOfInitializedPages = AlignToSize(sizeOfRawData, PELIB_PAGE_SIZE);
 	sizeOfValidPages = AlignToSize(virtualSize, PELIB_PAGE_SIZE);
+
+	// Calculate aligned size of the section. Mind invalid sizes.
+	// Example: 83e9cb2a6e78c742e1090292acf3c78baf76e82950d96548673795a1901db061
+	// * Size of the last section is 0xfffff000, sizeOfSection becomes 0
 	sizeOfSection = AlignToSize(virtualSize, optionalHeader.SectionAlignment);
+	if(sizeOfSection < virtualSize)
+		sizeOfSection = virtualSize;
 
 	// Get the range of the file containing valid data (aka nonzeros)
 	// Pointer to raw data is aligned down to the sector size
@@ -2549,7 +2561,7 @@ std::uint32_t PeLib::ImageLoader::captureImageSection(
 		if(pointerToRawData || isImageHeader)
 		{
 			// Fill all pages that contain data
-			while(pageOffset < sizeOfInitializedPages)
+			while(pageOffset < sizeOfInitializedPages && pageIndex < pages.size())
 			{
 				PELIB_FILE_PAGE & filePage = pages[pageIndex++];
 
@@ -2579,7 +2591,7 @@ std::uint32_t PeLib::ImageLoader::captureImageSection(
 		}
 
 		// Fill all pages that contain zeroed pages
-		while(pageOffset < sizeOfValidPages)
+		while(pageOffset < sizeOfValidPages && pageIndex < pages.size())
 		{
 			PELIB_FILE_PAGE & filePage = pages[pageIndex++];