EBiCS_Firmware icon indicating copy to clipboard operation
EBiCS_Firmware copied to clipboard

Potential stack-based buffer overflow in KM_901U_Service

Open VoodooChild99 opened this issue 1 month ago • 0 comments

Hi,

The function KM_901U_Service uses a buffer of 32 bytes to copy received messages: https://github.com/EBiCS/EBiCS_Firmware/blob/18b9437c088ac56e46872d28780e4cd06a264531/Src/display_kingmeter.c#L370

However, the length of KM_ctx->RxBuff is 64 bytes: https://github.com/EBiCS/EBiCS_Firmware/blob/18b9437c088ac56e46872d28780e4cd06a264531/Inc/display_kingmeter.h#L157

and the maximum length for DMA transfers is also 64 bytes: https://github.com/EBiCS/EBiCS_Firmware/blob/18b9437c088ac56e46872d28780e4cd06a264531/Src/display_kingmeter.c#L200-L216

This means that copying into KM_Message may overflow: https://github.com/EBiCS/EBiCS_Firmware/blob/18b9437c088ac56e46872d28780e4cd06a264531/Src/display_kingmeter.c#L374-L388

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!

VoodooChild99 avatar Nov 24 '25 14:11 VoodooChild99