otp: show error on invalid issuer, share logging strings
What the developer wrote
Authored by Mike Tolkachev
otp: show error on invalid issuer, share logging strings
What changed, and why it matters
This commit tightens error handling when decoding the 'issuer' field of an OTP (one-time password) setup URI. Previously, a malformed issuer string could fail to decode silently; now the device shows an error and cancels the operation instead of continuing with potentially bad data. It also makes minor logging message changes and adds an assertion for the label/issuer decode in the display code.
Treat as a routine hardening fix. No urgent action required. If auditing, verify that urldecode() can fail only on malformed input and that the new error path does not leak sensitive data in the errmsg returned to the host.
Security signals we found
Input validation added for OTP URI issuer field
Silent failure on malformed URL-decoding replaced with explicit error return
Debug assertion added for label/issuer URL-decoding in UI display path
Logging string changes only, no functional security mechanism added or removed
Evidence from the diff
The patch changes register_otp_string() in main/process/register_otp.c to check the return value of urldecode() on otp_ctx.issuer. If urldecode() fails, it now logs ‘Failed to decode otp issuer’, sets the error message, and returns CBOR_RPC_BAD_PARAMETERS. Previously the return value was ignored. In main/ui/otpauth.c, the urldecode() calls for label and issuer are wrapped with JADE_ASSERT(), turning a silent failure into a device-side assertion in debug builds. Several log strings were also shortened or shared. No memory corruption, cryptographic, or authentication bypass is evident in the diff.
Changed components
main/process/register_otp.cmain/ui/otpauth.cInspect captured patch +9 / −5
diff --git a/main/process/register_otp.c b/main/process/register_otp.c
index d86b4eb..aea7287 100644
--- a/main/process/register_otp.c
+++ b/main/process/register_otp.c
@@ -355,7 +355,7 @@ int register_otp_string(const char* otp_uri, const size_t uri_len, const char**
// Check keychain has seed data
if (keychain_get()->seed_len == 0) {
- JADE_LOGE("No wallet seed available. Wallet must be re-initialised from mnemonic.");
+ JADE_LOGE("No wallet seed available"); // Wallet must be re-initialised from mnemonic
*errmsg = "No wallet seed available";
await_error("Feature requires Jade wallet");
return CBOR_RPC_INTERNAL_ERROR;
@@ -365,13 +365,17 @@ int register_otp_string(const char* otp_uri, const size_t uri_len, const char**
char otp_name[OTP_MAX_NAME_LEN] = { 0 };
if (otp_ctx.issuer_len) {
// If have issuer, prefill otp_name with urldecoded version (truncates to fit if too long)
- urldecode(otp_ctx.issuer, otp_ctx.issuer_len, otp_name, sizeof(otp_name));
+ if (!urldecode(otp_ctx.issuer, otp_ctx.issuer_len, otp_name, sizeof(otp_name))) {
+ JADE_LOGE("Failed to decode otp issuer");
+ *errmsg = "Failed to decode otp issuer";
+ return CBOR_RPC_BAD_PARAMETERS;
+ }
// Ensure prefilled name is valid to use as storage key (eg. strip out any invalid chars)
storage_key_name_make_valid(otp_name);
}
if (!get_otp_data_from_kb(otp_name, sizeof(otp_name), NULL, 0, NULL)) {
// User abandoned
- JADE_LOGW("User abandoned (entering otp name)");
+ JADE_LOGW("User abandoned entering otp name");
*errmsg = "User abandoned entering otp name";
return CBOR_RPC_USER_CANCELLED;
}
diff --git a/main/ui/otpauth.c b/main/ui/otpauth.c
index fe2aa65..9e296e3 100644
--- a/main/ui/otpauth.c
+++ b/main/ui/otpauth.c
@@ -76,7 +76,7 @@ static gui_activity_t* make_otp_details_activities(const otpauth_ctx_t* ctx, con
if (ctx->label && ctx->label_len) {
// urldecode the label string - use font with no messed-with characters
- urldecode(ctx->label, ctx->label_len, display_str, sizeof(display_str));
+ JADE_ASSERT(urldecode(ctx->label, ctx->label_len, display_str, sizeof(display_str)));
} else {
const int ret = snprintf(display_str, sizeof(display_str), "<None>");
JADE_ASSERT(ret > 0 && ret < sizeof(display_str));
@@ -98,7 +98,7 @@ static gui_activity_t* make_otp_details_activities(const otpauth_ctx_t* ctx, con
if (ctx->issuer && ctx->issuer_len) {
// urldecode the issuer string - use font with no messed-with characters
- urldecode(ctx->issuer, ctx->issuer_len, display_str, sizeof(display_str));
+ JADE_ASSERT(urldecode(ctx->issuer, ctx->issuer_len, display_str, sizeof(display_str)));
} else {
const int ret = snprintf(display_str, sizeof(display_str), "<None>");
JADE_ASSERT(ret > 0 && ret < sizeof(display_str));
Why this scored 22/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.