Fix Thumb-2 out-of-bounds read during lifting

Fix for sentry crash BINARYNINJA-89

Author: zznop

arch/armv7/thumb2_disasm/arch_thumb2.cpp

@@ -98,9 +98,11 @@ class Thumb2Architecture: public ArmCommonArchitecture
 		return "thumbv7-none-none";
 	}
 
-	void populateDecomposeRequest(decomp_request *req, const uint8_t *data, size_t len,
+	bool populateDecomposeRequest(decomp_request *req, const uint8_t *data, size_t len,
 		uint64_t addr, int inIfThen, int inIfThenLast)
 	{
+		if (!data || len < 2)
+			return false;
 		req->instr_word16 = 0;
 		req->instr_word32 = 0;
 		if(m_endian == LittleEndian) {
@@ -122,14 +124,17 @@ class Thumb2Architecture: public ArmCommonArchitecture
 		req->inIfThenLast = inIfThenLast;
 		req->carry_in = 0;
 		req->addr = (uint32_t)addr;
+		return true;
 	}
 
 	virtual bool Disassemble(const uint8_t* data, uint64_t addr, size_t maxLen, decomp_result& result)
 	{
 		(void)addr;
 		(void)maxLen;
 		decomp_request request;
-		populateDecomposeRequest(&request, data, maxLen, addr, IFTHEN_UNKNOWN, IFTHENLAST_UNKNOWN);
+
+		if (!populateDecomposeRequest(&request, data, maxLen, addr, IFTHEN_UNKNOWN, IFTHENLAST_UNKNOWN))
+			return false;
 
 		memset(&result, 0, sizeof(result));
 		if (thumb_decompose(&request, &result) != STATUS_OK)
@@ -176,7 +181,8 @@ class Thumb2Architecture: public ArmCommonArchitecture
 		decomp_request request;
 		decomp_result decomp;
 
-		populateDecomposeRequest(&request, data, maxLen, addr, IFTHEN_UNKNOWN, IFTHENLAST_UNKNOWN);
+		if (!populateDecomposeRequest(&request, data, maxLen, addr, IFTHEN_UNKNOWN, IFTHENLAST_UNKNOWN))
+			return false;
 
 		if (thumb_decompose(&request, &decomp) != STATUS_OK)
 			return false;
@@ -460,7 +466,8 @@ class Thumb2Architecture: public ArmCommonArchitecture
 		decomp_request request;
 		decomp_result decomp;
 
-		populateDecomposeRequest(&request, data, len, addr, IFTHEN_UNKNOWN, IFTHENLAST_UNKNOWN);
+		if (!populateDecomposeRequest(&request, data, len, addr, IFTHEN_UNKNOWN, IFTHENLAST_UNKNOWN))
+			return false;
 
 		if (thumb_decompose(&request, &decomp) != STATUS_OK)
 			return false;
@@ -1759,7 +1766,8 @@ class Thumb2Architecture: public ArmCommonArchitecture
 		decomp_request request;
 		decomp_result decomp;
 
-		populateDecomposeRequest(&request, data, len, addr, IFTHEN_NO, IFTHENLAST_NO);
+		if (!populateDecomposeRequest(&request, data, len, addr, IFTHEN_NO, IFTHENLAST_NO))
+			return false;
 
 		if (thumb_decompose(&request, &decomp) != STATUS_OK)
 			return false;
@@ -1792,14 +1800,19 @@ class Thumb2Architecture: public ArmCommonArchitecture
 
 			for (size_t i = 0; i < instrCount; i++)
 			{
+				if (offset >= len || (len - offset) < 2)
+					return false;
+
 				bool isTrue = (i == 0) || (((mask >> (4 - i)) & 1) == (cond & 1));
+				size_t remainingLen = len - offset;
 
-				populateDecomposeRequest(&request, data+offset, len-offset, addr+offset,
-					IFTHEN_YES, ((i + 1) >= instrCount) ? IFTHENLAST_YES : IFTHENLAST_NO);
+				if (!populateDecomposeRequest(&request, data+offset, remainingLen, addr+offset,
+					IFTHEN_YES, ((i + 1) >= instrCount) ? IFTHENLAST_YES : IFTHENLAST_NO))
+					return false;
 
 				if (thumb_decompose(&request, &decomp) != STATUS_OK)
 					return false;
-				if ((offset + (decomp.instrSize / 8)) > len)
+				if ((decomp.instrSize / 8) > remainingLen)
 					return false;
 				if ((decomp.status & STATUS_UNDEFINED) || (!decomp.format))
 					return false;

defaultarch.cpp

@@ -796,16 +796,24 @@ bool Architecture::DefaultLiftFunction(LowLevelILFunction* function, FunctionLif
 				if (buffer.GetLength() == 0)
 					buffer = data->ReadBuffer(i->GetStart(), i->GetEnd() - i->GetStart());
 
-				if (addr < i->GetStart() || addr >= (i->GetStart() + buffer.GetLength()))
+				uint64_t blockStart = i->GetStart();
+				size_t bufferLen = buffer.GetLength();
+				if (addr < blockStart || (addr - blockStart) >= bufferLen)
 				{
-					// Instruction data not found, emit undefined IL instruction
 					function->AddInstruction(function->AddExpr(LLIL_UNDEF, 0, 0));
 					logger->LogDebug("Instruction data not found, inserted LLIL_UNDEF at %#" PRIx64, addr);
 					break;
 				}
 
-				len = (i->GetStart() + buffer.GetLength()) - addr;
-				opcode = (const uint8_t*)buffer.GetDataAt(addr - i->GetStart());
+				size_t bufferOffset = static_cast<size_t>(addr - blockStart);
+				len = bufferLen - bufferOffset;
+				opcode = (const uint8_t*)buffer.GetDataAt(bufferOffset);
+				if (!opcode)
+				{
+					function->AddInstruction(function->AddExpr(LLIL_UNDEF, 0, 0));
+					logger->LogDebug("Instruction data not found, inserted LLIL_UNDEF at %#" PRIx64, addr);
+					break;
+				}
 			}
 
 			size_t instrCountBefore = function->GetInstructionCount();