What the developer wrote
Authored by ww3512687
reivew device setting
What changed, and why it matters
This commit fixes three small but real code-quality issues in the Keystone 3 hardware wallet firmware: a global device-settings variable was made private to one file, a memory leak was plugged in the log-reading code, and screen brightness is now capped at a safe maximum. The commit message is just a typo-filled 'reivew device setting', so the vendor did not describe these as security fixes. The changes are defensive hardening rather than an obvious exploit fix.
Treat as a routine hardening/maintenance patch. Review whether `MAX_BRIGHT` is defined at a sensible hardware-safe level, and confirm `EXT_FREE` is the correct allocator-matching free macro. No urgent security response is indicated by the diff alone.
Security signals we found
Global variable reduced to file-static scope
Input value clamped to defined maximum before hardware/backing-store use
Memory freed on previously-missed error/exit paths
Commit message does not mention security
Evidence from the diff
The diff makes three changes: (1) g_deviceSettings is changed from global to static in src/device_settings.c, reducing symbol visibility. (2) SetBright() now clamps its argument to MAX_BRIGHT before calling SetLcdBright() and storing it, fixing a parameter-name typo (bight -> bright) in the process. (3) Two missing EXT_FREE() calls are added: one for a JSON string in DeviceSettingsInit() and one for originalData in FindLogOffsetAddr() after an error path. A redundant empty CheckAllFF block is also removed. No CVE, advisory, or researcher attribution is present in the supplied materials.
Changed components
src/device_settings.csrc/device_setting.hsrc/utils/log/log.cInspect captured patch +13 / −8
diff --git a/src/device_setting.h b/src/device_setting.h
index c5b9406..5582777 100644
--- a/src/device_setting.h
+++ b/src/device_setting.h
@@ -17,7 +17,7 @@ uint32_t GetSetupStep(void);
void SetSetupStep(uint32_t setupStep);
uint32_t GetBright(void);
-void SetBright(uint32_t bight);
+void SetBright(uint32_t bright);
uint32_t GetAutoLockScreen(void);
void SetAutoLockScreen(uint32_t autoLockScreen);
diff --git a/src/device_settings.c b/src/device_settings.c
index cbfca4c..33d9e47 100644
--- a/src/device_settings.c
+++ b/src/device_settings.c
@@ -90,7 +90,7 @@ typedef struct {
static BootParam_t g_bootParam;
static const char g_deviceSettingsVersion[] = "1.0.0";
-DeviceSettings_t g_deviceSettings;
+static DeviceSettings_t g_deviceSettings;
static const uint8_t g_integrityFlag[16] = {
0x01, 0x09, 0x00, 0x03,
0x01, 0x09, 0x00, 0x03,
@@ -142,7 +142,10 @@ void DeviceSettingsInit(void)
SaveDeviceSettingsSync();
}
- // init boot param
+ if (jsonString != NULL) {
+ EXT_FREE(jsonString);
+ }
+
InitBootParam();
}
@@ -160,8 +163,6 @@ void InitBootParam(void)
memcpy(g_bootParam.bootCheckFlag, g_integrityFlag, sizeof(bootParam.bootCheckFlag));
needSave = true;
}
- if (CheckAllFF(bootParam.recoveryModeSwitch, sizeof(bootParam.recoveryModeSwitch))) {
- }
if (needSave) {
SaveBootParam();
} else {
@@ -241,10 +242,13 @@ uint32_t GetBright(void)
return g_deviceSettings.bright;
}
-void SetBright(uint32_t bight)
+void SetBright(uint32_t bright)
{
- SetLcdBright(bight);
- g_deviceSettings.bright = bight;
+ if (bright > MAX_BRIGHT) {
+ bright = MAX_BRIGHT;
+ }
+ SetLcdBright(bright);
+ g_deviceSettings.bright = bright;
}
uint32_t GetAutoLockScreen(void)
diff --git a/src/utils/log/log.c b/src/utils/log/log.c
index cf7f197..4fd9189 100644
--- a/src/utils/log/log.c
+++ b/src/utils/log/log.c
@@ -344,6 +344,7 @@ static uint32_t FindLogOffsetAddr(void)
addr = addr + LOG_DATA_HEAD_SIZE + logData.length * 4;
}
//err, need erase the whole log flash zone.
+ EXT_FREE(originalData);
LogEraseSync();
return SPI_FLASH_ADDR_LOG;
}
Why this scored 28/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.