fix: update memory management in async tasks and GUI auth code handling
What the developer wrote
Authored by ww3512687
fix: update memory management in async tasks and GUI auth code handling
What changed, and why it matters
This firmware update fixes two categories of bugs in a cryptocurrency hardware wallet's background task system and web-authentication code display. First, it adds a flag so the background task knows whether it allocated a memory buffer itself (and must free it) or was given a pointer by the caller (which it must not free). Before this fix, the task could free memory it did not own, leading to crashes or corrupted memory. Second, it hardens the web-authentication code calculation by checking whether memory allocation succeeded, clearing the private RSA key from memory after use, and correctly freeing the generated auth code string. It also removes two lines that incorrectly set freed pointers to NULL inside a cleanup function. The changes are defensive and reduce the risk of memory corruption and sensitive key material lingering in RAM, but the commit message frames them as generic fixes rather than as a security advisory.
Treat this as a routine but worthwhile defensive patch. Review other async task callers to confirm none relied on the previous unconditional-free behavior, and verify that `free_ur_parse_multi_result`/`free_ur_parse_result` correctly null their internal `data` fields so the removed assignments are not needed. Consider whether the web-auth RSA key handling path needs additional audit for other missing error checks or secret scrubbing.
Security signals we found
Memory ownership flag added to async task structure to prevent freeing caller-owned buffers
Missing null check after SRAM_MALLOC for RSA key buffer is now present
Private RSA key material is explicitly cleared with memset_s before deallocation
Return value of GetWebAuthRsaKey is now checked and failures handled
Generated web auth code string is now freed after signal emission
Signal payload length changed to include null terminator
Removed post-free pointer assignments in web auth result deinitialization
Evidence from the diff
The patch modifies four files. In fetch_sensitive_data_task.c/h it introduces a shouldFree boolean in BackgroundAsync_t. AsyncExecute and AsyncDelayExecute (which SRAM_MALLOC and copy inData) set it to true; AsyncExecuteWithPtr (which receives an external pointer) sets it to false. The task loop now only calls SRAM_FREE(async->inData) when shouldFree is true, preventing a use-after-free/double-free when a caller passes a pointer it still owns. In gui_model.c, ModelCalculateWebAuthCode now checks the key allocation result, clears the key buffer with memset_s before freeing, checks the return value of GetWebAuthRsaKey, handles a NULL authCode from calculate_auth_code, and frees it with free_ptr_string after emitting the signal. It also changes the emitted length from strlen(authCode) to strlen(authCode)+1 so the null terminator is included. In gui_web_auth_result_widgets.c it removes g_urMultiResult->data = NULL and g_urResult->data = NULL comments/assignments before calling the respective free functions, which may have been masking or causing use-after-free issues if the free functions already clear the fields. No CVE, advisory, or researcher attribution is present in the supplied materials.
Changed components
src/tasks/fetch_sensitive_data_task.csrc/tasks/fetch_sensitive_data_task.hsrc/ui/gui_model/gui_model.csrc/ui/gui_widgets/gui_web_auth_result_widgets.cInspect captured patch +32 / −10
diff --git a/src/tasks/fetch_sensitive_data_task.c b/src/tasks/fetch_sensitive_data_task.c
index ec6c758..ceff915 100644
--- a/src/tasks/fetch_sensitive_data_task.c
+++ b/src/tasks/fetch_sensitive_data_task.c
@@ -48,6 +48,7 @@ int32_t AsyncExecute(BackgroundAsyncFunc_t func, const void *inData, uint32_t in
async.inData = SRAM_MALLOC(inDataLen);
memcpy(async.inData, inData, inDataLen);
async.inDataLen = inDataLen;
+ async.shouldFree = true;
}
PubBufferMsg(SENSITIVE_MSG_EXECUTE, &async, sizeof(BackgroundAsync_t));
return SUCCESS_CODE;
@@ -59,6 +60,7 @@ int32_t AsyncExecuteWithPtr(BackgroundAsyncFunc_t func, const void *inData)
async.func = func;
async.inData = (void *)inData;
async.inDataLen = 4;
+ async.shouldFree = false;
PubBufferMsg(SENSITIVE_MSG_EXECUTE, &async, sizeof(BackgroundAsync_t));
return SUCCESS_CODE;
}
@@ -78,6 +80,7 @@ int32_t AsyncDelayExecute(BackgroundAsyncFunc_t func, const void *inData, uint32
async.inData = SRAM_MALLOC(inDataLen);
memcpy(async.inData, inData, inDataLen);
async.inDataLen = inDataLen;
+ async.shouldFree = true;
}
PubBufferMsg(SENSITIVE_MSG_EXECUTE, &async, sizeof(BackgroundAsync_t));
return SUCCESS_CODE;
@@ -120,7 +123,7 @@ static void FetchSensitiveDataTask(void *argument)
if (async->func) {
async->func(async->inData, async->inDataLen);
}
- if (async->inData) {
+ if (async->shouldFree && async->inData) {
SRAM_FREE(async->inData);
}
}
@@ -148,4 +151,4 @@ static void FetchSensitiveDataTask(void *argument)
SRAM_FREE(rcvMsg.buffer);
}
}
-}
\ No newline at end of file
+}
diff --git a/src/tasks/fetch_sensitive_data_task.h b/src/tasks/fetch_sensitive_data_task.h
index 16f8b68..cda88a1 100644
--- a/src/tasks/fetch_sensitive_data_task.h
+++ b/src/tasks/fetch_sensitive_data_task.h
@@ -13,6 +13,7 @@ typedef struct {
void *inData;
uint32_t inDataLen;
uint32_t delay;
+ bool shouldFree;
} BackgroundAsync_t;
typedef struct {
diff --git a/src/ui/gui_model/gui_model.c b/src/ui/gui_model/gui_model.c
index 0037b17..e267669 100644
--- a/src/ui/gui_model/gui_model.c
+++ b/src/ui/gui_model/gui_model.c
@@ -1093,10 +1093,32 @@ static int32_t ModelCalculateWebAuthCode(const void *inData, uint32_t inDataLen)
SetLockScreen(false);
#ifndef COMPILE_SIMULATOR
uint8_t *key = SRAM_MALLOC(WEB_AUTH_RSA_KEY_LEN);
- GetWebAuthRsaKey(key);
+ if (key == NULL) {
+ char *authCode = "";
+ GuiApiEmitSignal(SIG_WEB_AUTH_CODE_SUCCESS, authCode, strlen(authCode) + 1);
+ SetLockScreen(enable);
+ return SUCCESS_CODE;
+ }
+ int32_t ret = GetWebAuthRsaKey(key);
+ if (ret != SUCCESS_CODE) {
+ memset_s(key, WEB_AUTH_RSA_KEY_LEN, 0, WEB_AUTH_RSA_KEY_LEN);
+ SRAM_FREE(key);
+ char *authCode = "";
+ GuiApiEmitSignal(SIG_WEB_AUTH_CODE_SUCCESS, authCode, strlen(authCode) + 1);
+ SetLockScreen(enable);
+ return SUCCESS_CODE;
+ }
char *authCode = calculate_auth_code(inData, key, 512, &key[512], 512);
+ bool shouldFreeAuthCode = authCode != NULL;
+ memset_s(key, WEB_AUTH_RSA_KEY_LEN, 0, WEB_AUTH_RSA_KEY_LEN);
SRAM_FREE(key);
- GuiApiEmitSignal(SIG_WEB_AUTH_CODE_SUCCESS, authCode, strlen(authCode));
+ if (authCode == NULL) {
+ authCode = "";
+ }
+ GuiApiEmitSignal(SIG_WEB_AUTH_CODE_SUCCESS, authCode, strlen(authCode) + 1);
+ if (shouldFreeAuthCode) {
+ free_ptr_string(authCode);
+ }
#else
uint8_t *entropy;
uint8_t entropyLen;
@@ -1105,7 +1127,7 @@ static int32_t ModelCalculateWebAuthCode(const void *inData, uint32_t inDataLen)
// GuiApiEmitSignal(SIG_SETTING_CHANGE_PASSWORD_FAIL, &ret, sizeof(ret));
char *authCode = "12345Yyq";
- GuiEmitSignal(SIG_WEB_AUTH_CODE_SUCCESS, authCode, strlen(authCode));
+ GuiEmitSignal(SIG_WEB_AUTH_CODE_SUCCESS, authCode, strlen(authCode) + 1);
#endif
SetLockScreen(enable);
return SUCCESS_CODE;
diff --git a/src/ui/gui_widgets/gui_web_auth_result_widgets.c b/src/ui/gui_widgets/gui_web_auth_result_widgets.c
index 0c99db6..25daf33 100644
--- a/src/ui/gui_widgets/gui_web_auth_result_widgets.c
+++ b/src/ui/gui_widgets/gui_web_auth_result_widgets.c
@@ -237,12 +237,8 @@ void GuiWebAuthResultAreaDeInit()
GuiWebAuthResultHidePending();
if (g_urResult != NULL) {
if (g_isMulti) {
- // has already free
- g_urMultiResult->data = NULL;
free_ur_parse_multi_result(g_urMultiResult);
} else {
- // has already free
- g_urResult->data = NULL;
free_ur_parse_result(g_urResult);
}
g_urResult = NULL;
@@ -327,4 +323,4 @@ void GuiWebAuthShowAuthCode(char *authCode)
g_authCode = authCode;
GuiWebAuthResultRenderAuthCode(g_WebAuthResultTileView.result);
GuiWebAuthResultHidePending();
-}
\ No newline at end of file
+}
Why this scored 59/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.