Potential stack-based buffer overflow in KM_901U_Service

[VoodooChild99]

Hi,

The function KM_901U_Service uses a buffer of 32 bytes to copy received messages:

EBiCS_Firmware/Src/display_kingmeter.c

Line 370 in 18b9437

static uint8_t KM_Message[32];

However, the length of KM_ctx->RxBuff is 64 bytes:

EBiCS_Firmware/Inc/display_kingmeter.h

Line 157 in 18b9437

uint8_t RxBuff[KM_MAX_RXBUFF];

and the maximum length for DMA transfers is also 64 bytes:

EBiCS_Firmware/Src/display_kingmeter.c

Lines 200 to 216 in 18b9437

	#if (DISPLAY_TYPE == DISPLAY_TYPE_KINGMETER_618U)
	//Start UART with DMA
	if (HAL_UART_Receive_DMA(&huart1, (uint8_t *)KM_ctx->RxBuff, KM_MAX_RXBUFF) != HAL_OK)
	{
	Error_Handler();
	}
	#endif
	#if (DISPLAY_TYPE == DISPLAY_TYPE_KINGMETER_901U|| DISPLAY_TYPE & DISPLAY_TYPE_DEBUG)
	//Start UART with DMA
	if (HAL_UART_Receive_DMA(&huart1, (uint8_t *)KM_ctx->RxBuff, 64) != HAL_OK)
	{
	Error_Handler();
	}
	#endif
	// HAL_UART_Transmit_DMA(&huart1, (uint8_t *)&buffer, KM_MAX_RXBUFF);
	}

This means that copying into KM_Message may overflow:

EBiCS_Firmware/Src/display_kingmeter.c

Lines 374 to 388 in 18b9437

	if(recent_pointer_position>last_pointer_position){
	Rx_message_length=recent_pointer_position-last_pointer_position;
	//printf_("groesser %d, %d, %d \n ",recent_pointer_position,last_pointer_position, Rx_message_length);
	//HAL_GPIO_WritePin(LED_GPIO_Port, LED_Pin, GPIO_PIN_SET);
	memcpy(KM_Message,KM_ctx->RxBuff+last_pointer_position,Rx_message_length);
	//HAL_UART_Transmit(&huart3, (uint8_t *)&KM_Message, Rx_message_length,50);
	}
	else {
	Rx_message_length=recent_pointer_position+64-last_pointer_position;
	memcpy(KM_Message,KM_ctx->RxBuff+last_pointer_position,64-last_pointer_position);
	memcpy(KM_Message+64-last_pointer_position,KM_ctx->RxBuff,recent_pointer_position);
	// HAL_GPIO_WritePin(LED_GPIO_Port, LED_Pin, GPIO_PIN_RESET);
	}

In my case, the memcpy corrupts htim3->Instance and later causes a data abort exception in TIM3_IRQHandler.

Therefore, I believe the length of KM_Message should be set to 64 bytes to address the above issue:

diff --git a/Src/display_kingmeter.c b/Src/display_kingmeter.c
index 7cbca45..6e5b154 100644
--- a/Src/display_kingmeter.c
+++ b/Src/display_kingmeter.c
@@ -367,7 +367,7 @@ static void KM_901U_Service(KINGMETER_t* KM_ctx)
 
     static uint8_t  TxCnt;
     static uint8_t  Rx_message_length;
-    static uint8_t  KM_Message[32];
+    static uint8_t  KM_Message[64];
 
     recent_pointer_position = 64-DMA1_Channel5->CNDTR;
 

Looking for your reply!