diff --git a/docs/AUDIT_2026-07.md b/docs/AUDIT_2026-07.md new file mode 100644 index 0000000..86b7f84 --- /dev/null +++ b/docs/AUDIT_2026-07.md @@ -0,0 +1,288 @@ +# Independent code audit — 2026-07-27 + +Multi-agent review of the AmiAuth codebase at commit `e9e1e78` (main), covering +core crypto/vault (`src/core/`), CLI (`src/cli/`), GUI (`src/gui/`), the Amiga +platform/entropy layer (`src/amiga/`), and the QR decoder integration wrapper +(`src/qr/qr.c`, `src/amiga/qrimage.c`). Vendored third-party quirc internals +were excluded per [THIRDPARTY.md](../THIRDPARTY.md). + +Method: six independent reviewers each covered one dimension of the codebase; +every finding was then checked by two independent adversarial verifiers before +being included here. 17 candidate findings were raised, 9 were refuted as +false positives on verification, 8 survived. See "Reviewed, no issues found" +below for the areas that turned up nothing. + +Track fixes against this list; update or remove entries here as they're +resolved rather than leaving them to go stale. + +**Status: all 8 findings fixed** (2026-07-27), verified against the host test +suite (`make test`, 292/292 passing) and clean `make m68k-docker`/`make +gui-docker` cross-builds. Each finding below records what was fixed and, where +the audit's own suggested fix direction wasn't the right call for this +codebase, why. + +## Executive summary + +Overall code health is good: no memory-corruption bugs, no buffer overflows, +and no violations of the frozen vault format or the 68000/zero-dependency +baseline were found. The codebase shows a consistent, deliberate security +posture in its primary paths (vault key zeroing on lock/quit, format-version +rejection, interactive-only passphrase entry). The findings that did surface +cluster around two themes: + +1. **Inconsistent secret-hygiene discipline** — the project's own "zero key + material after use" practice, well-applied in `vault.c`, is not carried + through to the hot, high-frequency HMAC/PBKDF2 primitives or to one CLI + error path, leaving passphrase- and secret-derived bytes on the stack + longer than necessary on a platform with no memory protection. +2. **Weak trust assumptions in network/UI-adjacent code** — the SNTP client + accepts a time reply from any host, and the GUI's passphrase entry + silently drops the keystroke-timing entropy source the design docs call + the primary defence on quiescent/emulated hardware. + +None of the findings are exploitable for remote compromise or vault +corruption; the highest-severity items require either local code execution +(already a stated "no memory protection" edge in the project's own threat +model, see [SECURITY.md](SECURITY.md)) or LAN/on-path network position. + +## Findings + +### High + +#### 1. SNTP reply accepted from any source, not just the queried server + +- **File:** `src/amiga/sntp.c:71` +- **Status:** fixed — the UDP socket now `connect()`s to the resolved server + address before exchanging packets (`send()`/`recv()` instead of + `sendto()`/`recvfrom()`), so the stack drops any reply not actually from + that peer. The "sanity bound on offset magnitude" part of the original + suggestion was deliberately **not** applied: `docs/CLOCK.md` documents that + RTC-less machines can legitimately need decades-large corrections, so + bounding the offset would reject valid syncs. + +`recvfrom()` is called with `NULL` for the source-address arguments, and the +socket is never `connect()`-ed, so any UDP datagram delivered to the ephemeral +port is treated as the server's reply. + +*Failure scenario:* An attacker on the same LAN/WiFi, a compromised router, or +an off-path attacker racing the real server can inject a forged SNTP response +before the genuine one (or before the 5s timeout) arrives. The only anti-spoof +check (`orig != t1`, where `t1` is `time(NULL)` sent in cleartext) is trivially +satisfied by anyone who can see the request or guess the current second. The +forged offset is applied with no sanity bound and the clock is marked +`CLOCK_SYNCED` (trusted/green in the UI), silently desynchronizing the +corrected UTC used for TOTP generation and validation. + +*Fix direction:* Validate the reply's source address/port against the +resolved server before accepting it, and add a sanity bound on the accepted +offset magnitude; consider a less guessable originate-timestamp nonce. + +#### 2. GUI passphrase entry never feeds the entropy pool + +- **File:** `src/gui/main.c:729` +- **Status:** fixed — added `amiga_stir_keystroke()` (exported from + `src/amiga/random.c`/`entropy.h`) and called it on every + `WMHI_VANILLAKEY` event in `passphrase_request()`, mirroring the CLI's + per-keystroke `stir_eclock()` call. `amiga_entropy_stir()` itself is left + in place — it's public API for folding externally-supplied samples, a + distinct role from the new keystroke-timing hook. + +`passphrase_request()` reads keystrokes via `IDCMP_VANILLAKEY` in a ReAction +event loop and never calls `amiga_entropy_stir()` (confirmed dead code, never +called anywhere) or otherwise stirs timing into the RNG state, unlike the +CLI's `amiga_read_passphrase()`, which calls `stir_eclock()` per keystroke. + +*Failure scenario:* [SECURITY.md](SECURITY.md) states keystroke timing during +passphrase entry is "the main real source at the moment it matters (vault +creation)." GUI first-run vault creation calls `passphrase_request()` for the +new passphrase, then generates the vault salt via `amiga_random()` with no +timing contribution in between. Since the GUI is plausibly the primary +first-run UX, GUI-created vaults fall back to `amiga_random()`'s passive +entropy (EClock jitter, `DateStamp`, `AvailMem`, addresses) — which the same +doc says yields "little entropy" on a quiescent 68000 and "almost none" under +a deterministic emulator — silently, with no user-visible warning. + +*Fix direction:* Wire keystroke-timing stirring into the GUI's +`IDCMP_VANILLAKEY` handler the same way the CLI's RAW-read loop does, and +delete or use `amiga_entropy_stir()`. + +### Medium + +#### 3. `passphrase_request()` leaks its ReAction gadget tree if window creation fails + +- **File:** `src/gui/main.c:729` +- **Status:** fixed — `passphrase_request()`, `uri_request()`, `edit_request()`, + and `secret_meta_request()` (the four `window.class` requesters sharing this + pattern) now `DisposeObject(layoutobj)` on the `!win` path. `qr_file_request()` + and `vault_location_request()` were checked too but use ASL requesters, not + `window.class`, so they don't share this leak. + +`layoutobj` (owning `labelobj`, `maskobj`, `okobj`, `cancelobj`, `buttons`) is +built up front; if `NewObject(WINDOW_GetClass(), ...)` returns `NULL`, the +function does `if (!win) return 0;` with no `DisposeObject(layoutobj)`. +Ownership of the layout only transfers to the window object on success, so +failure orphans the whole tree. The same pattern recurs at other requester +call sites in the file (`uri_request` and others), so this is systemic rather +than a one-off. + +*Failure scenario:* Under memory pressure, `NewObject(WINDOW_GetClass(), ...)` +can fail while earlier, smaller allocations (labels, buttons, layout) already +succeeded. Since this requester runs on every unlock attempt and every +first-run passphrase entry, repeated failures under sustained memory pressure +leak the gadget tree each time, worsening the very condition that caused the +failure. + +*Fix direction:* On `!win`, explicitly `DisposeObject(layoutobj)` (and apply +the same guard at the other requester call sites showing the identical +pattern). + +#### 4. HMAC init leaves padded key and ipad unzeroed on the stack + +- **File:** `src/core/hmac.c:11` (also lines 67, 123) +- **Status:** fixed — all three `hmac_*_init()` functions now `memset` their + `k[]`/`ipad[]` stack locals to zero before returning. `ctx->opad` is left + alone: it's still needed by the matching `_final()` call, and zeroing it as + part of a context-teardown step would be a broader API change (an explicit + teardown function doesn't currently exist) — out of scope for this fix. + +`hmac_sha1_init`/`hmac_sha256_init`/`hmac_sha512_init` build the block-padded +key (`k[]`) and `ipad[]` (`k` XOR `0x36`) as stack locals and never `memset` +them to zero before returning, unlike `vault.c`'s `derive_keys()`, which +explicitly zeroes its own scratch buffer after use. + +*Failure scenario:* These functions run on every PBKDF2 iteration (with the +raw passphrase as key), every vault MAC computation (with `mac_key`), every +TOTP/HOTP code render (with the raw secret), and every DRBG step. Given +AmigaOS's lack of memory protection (acknowledged in +[SECURITY.md](SECURITY.md)), any other running process can read this +process's stack and recover recently-used key/secret material long after the +operation completed, undermining the documented "key material is zeroed on +lock and quit" mitigation, which this code path runs independently of and far +more frequently than. + +*Fix direction:* Zero `k[]` and `ipad[]` (and the context's `opad`/`inner` +state) before returning/on context teardown, matching the zeroing discipline +already used in `vault.c`. + +### Low + +#### 5. PBKDF2 per-iteration HMAC context and U/T buffers never zeroed + +- **File:** `src/core/pbkdf2.c:15` +- **Status:** fixed — `pbkdf2_block()` now zeroes `idx`, `u`, `t`, and the + `hmac_sha1_ctx c` before returning; `pbkdf2_hmac_sha1()` also zeroes its + `block` scratch buffer after each iteration's `memcpy` into `out`. + +`pbkdf2_block()`'s stack locals `idx[4]`, `u[]`, `t[]`, and `hmac_sha1_ctx c` +(whose `opad` is derived from the passphrase) persist unzeroed across up to +the configured iteration count and after the function returns. + +*Failure scenario:* Same exposure class as finding 4 — a concurrently running +process on the same non-protected AmigaOS instance can scan stack memory +after a vault unlock/creation and recover passphrase-derived intermediate KDF +state. + +*Fix direction:* Zero `idx`, `u`, `t`, and the HMAC context before +`pbkdf2_block()`/`pbkdf2_hmac_sha1()` return. + +#### 6. `parse_payload()` does not validate the per-account `digits` byte + +- **File:** `src/core/vault.c:170` +- **Status:** fixed — `parse_payload()` now rejects (`VAULT_ERR_FORMAT`) any + `digits` value AmiAuth itself never writes: exactly `STEAM_CODE_DIGITS` (5) + for Steam Guard accounts, 6-8 for everything else (matching what + `uri.c`'s otpauth:// parser and the GUI's `edit_request()` validation + actually allow to be written — the GUI permits 7 in addition to the + documented "6 or 8", not just those two values, so the check uses the + inclusive range rather than an exact `alg`-style enum match). + +`a.digits = in[p++]` is read with no range check, unlike the adjacent +`alg`/`type` fields two lines later, which are explicitly rejected with +`VAULT_ERR_FORMAT` if out of range. [VAULT_FORMAT.md](VAULT_FORMAT.md) +documents `digits` as "6 or 8." + +*Failure scenario:* A corrupted or hand-edited vault with `digits=0` or +`digits=200` loads successfully (`VAULT_OK`). No crash or overflow occurs — +`otp_render()`/`hotp()` independently clamp out-of-range values to +`OTP_DEFAULT_DIGITS` and the code buffer is sized safely — but the vault +layer provides no defense-in-depth for this field the way it does for its +siblings, and a corrupted `digits` byte silently produces a +plausible-but-wrong code length instead of being reported as a format error. + +*Fix direction:* Validate `digits` (e.g., 6/7/8, or the accepted set +including 5 for Steam Guard) in `parse_payload()` and return +`VAULT_ERR_FORMAT` on violation, consistent with the `alg`/`type` checks. + +#### 7. Decoded secret not zeroed on `open_vault()` failure in `cmd_add()` + +- **File:** `src/cli/main.c` (`cmd_add()`) +- **Status:** fixed — the `open_vault()` failure branch now + `memset(&acct, 0, sizeof(acct))` before returning, matching every other + exit path in the function and `cmd_add_secret()`'s equivalent branch. + +If `open_vault()` fails after `otpauth_parse()` has already decoded the +Base32 secret into the stack-local `acct`, `cmd_add()` returns immediately +without `memset(&acct, 0, sizeof(acct))`, unlike every other exit path in the +same function and unlike the identical failure branch in the sibling +`cmd_add_secret()`, which does zero `acct` in this case. + +*Failure scenario:* Running `AmiAuth ADD "otpauth://..."` against an +encrypted vault with a mistyped passphrase (or any vault I/O error) leaves +the freshly decoded TOTP/HOTP secret in stack memory, unscrubbed, contrary to +the project's stated "key material is zeroed" practice. + +*Fix direction:* Add the same `memset(&acct, 0, sizeof(acct))` to this +early-return branch that `cmd_add_secret()` already has. + +#### 8. `qr_decode_gray()` dereferences `uri` before validating it is non-NULL + +- **File:** `src/qr/qr.c:131` +- **Status:** fixed — the argument-validation check now runs before the + `uri[0] = '\0'` write, so a `NULL uri` returns `QR_ERR_ARGS` as documented + instead of crashing. + +Line 131 writes `uri[0] = '\0'` when `cap > 0`, before the `!uri` check on +line 133 (the same check that exists specifically to catch a NULL `uri`, per +the documented `QR_ERR_ARGS` contract). + +*Failure scenario:* A caller invoking `qr_decode_gray(gray, w, h, NULL, cap)` +with `cap > 0` crashes on the NULL dereference instead of receiving the +documented `QR_ERR_ARGS`. Not reachable via the current single production +caller (`src/gui/main.c`, which always passes a valid static buffer) and +untested by the current unit tests, but it inverts the function's own +argument-validation contract. + +*Fix direction:* Reorder the argument-validation check before the +`uri[0] = '\0'` write. + +## Reviewed, no issues found + +The following areas were reviewed against the stated hard constraints (68000 +baseline, zero mandatory runtime dependencies, interactive-only passphrase, +frozen vault format, ≤4 KB stack discipline, and standard memory-safety / +timing-side-channel concerns) with no additional findings surviving review: + +- **Crypto primitives** (SHA-1/256/512, HMAC, PBKDF2, DRBG core logic, + TOTP/HOTP code generation) — beyond the stack-zeroing gaps above (findings + 4–5), no buffer overflows, integer overflows, or weak/predictable + randomness in the algorithms themselves. +- **Vault security** (format parsing, encryption/MAC, load/save, lock/quit + key zeroing) — beyond the `digits` validation gap above (finding 6), no + format-compatibility breaks, no stack-allocation-of-large-objects + regressions, no MAC/comparison timing issues found. +- **CLI** (`src/cli/main.c` and friends) — passphrase handling confirmed + interactive-only with no CLI-arg/env/file/commodity-port hatch; beyond the + one unzeroed error path above (finding 7), no argument-parsing or + I/O-handling defects found. +- **GUI** (`src/gui/main.c`, ReAction/BOOPSI usage) — beyond the entropy and + leak findings above (findings 2–3), no other memory-safety or logic + defects found in dialog/window handling. +- **Amiga platform/entropy layer** (`src/amiga/random.c`, `entropy.c`, + clock/SNTP glue) — beyond the SNTP source-validation gap above (finding + 1), entropy gathering and clock-offset application logic otherwise + consistent with documented design ([CLOCK.md](CLOCK.md), + [SECURITY.md](SECURITY.md)). +- **QR wrapper integration** (`src/qr/qr.c`, `src/amiga/qrimage.c`) — beyond + the argument-check ordering above (finding 8), no issues found in + AmiAuth's own integration code calling into vendored quirc; quirc's + internals were out of scope per [THIRDPARTY.md](../THIRDPARTY.md). diff --git a/src/amiga/entropy.h b/src/amiga/entropy.h index 4ae73f8..9776ea3 100644 --- a/src/amiga/entropy.h +++ b/src/amiga/entropy.h @@ -16,6 +16,12 @@ int amiga_random(uint8_t *buf, size_t n); /* Fold an application-supplied sample into the entropy pool. */ void amiga_entropy_stir(const void *p, size_t n); +/* Fold a fresh E-clock reading into the entropy pool — call this on every + * keystroke of any UI's own passphrase input loop (amiga_read_passphrase() + * already does this internally; front ends with their own event-driven + * keystroke handling, e.g. the GUI, must call it explicitly per keystroke). */ +void amiga_stir_keystroke(void); + /* Prompt and read a passphrase with no echo, using RAW console mode. Each * keystroke's arrival time is stirred into the entropy pool. Returns 0 on * success, -1 if there is no interactive console or on error. */ diff --git a/src/amiga/random.c b/src/amiga/random.c index 12a5efb..54ac13c 100644 --- a/src/amiga/random.c +++ b/src/amiga/random.c @@ -107,6 +107,12 @@ static void stir_eclock(void) sha1_update(&g_pool, &ev, sizeof ev); } +void amiga_stir_keystroke(void) +{ + pool_ensure(); + if (timer_ready()) stir_eclock(); +} + /* Fold volatile system state that varies run-to-run. */ static void stir_system_state(void) { diff --git a/src/amiga/sntp.c b/src/amiga/sntp.c index 5717f25..b8de621 100644 --- a/src/amiga/sntp.c +++ b/src/amiga/sntp.c @@ -62,14 +62,18 @@ int clock_sntp_sync(clock_ctx *c, const char *server) tv.tv_usec = 0; setsockopt(sock, SOL_SOCKET, SO_RCVTIMEO, (char *)&tv, sizeof(tv)); + /* Bind the datagram socket to the resolved server address/port so the + * stack drops any reply not actually from the server we queried — + * without this, recvfrom() would accept a UDP packet from anyone. */ + if (connect(sock, (struct sockaddr *)&addr, sizeof(addr)) < 0) + break; + t1 = (uint64_t)time(NULL); /* local send time */ clock_ntp_build_request(req, t1); - if (sendto(sock, (char *)req, NTP_PACKET_SIZE, 0, - (struct sockaddr *)&addr, sizeof(addr)) != NTP_PACKET_SIZE) + if (send(sock, (char *)req, NTP_PACKET_SIZE, 0) != NTP_PACKET_SIZE) break; - if (recvfrom(sock, (char *)resp, NTP_PACKET_SIZE, 0, NULL, NULL) - != NTP_PACKET_SIZE) + if (recv(sock, (char *)resp, NTP_PACKET_SIZE, 0) != NTP_PACKET_SIZE) break; t4 = (uint64_t)time(NULL); /* local receive time */ diff --git a/src/cli/main.c b/src/cli/main.c index ba739ed..539a0e1 100644 --- a/src/cli/main.c +++ b/src/cli/main.c @@ -656,7 +656,11 @@ static int cmd_add(const char *path, const char *uri) return 2; } rc = open_vault(&v, path); - if (rc != VAULT_OK) { fprintf(stderr, "AmiAuth: %s\n", vault_err(rc)); return 2; } + if (rc != VAULT_OK) { + memset(&acct, 0, sizeof(acct)); + fprintf(stderr, "AmiAuth: %s\n", vault_err(rc)); + return 2; + } rc = vault_add(&v, &acct); if (rc == VAULT_OK) rc = save_vault(&v, path); diff --git a/src/core/hmac.c b/src/core/hmac.c index 4335710..66883df 100644 --- a/src/core/hmac.c +++ b/src/core/hmac.c @@ -30,6 +30,9 @@ void hmac_sha1_init(hmac_sha1_ctx *ctx, const uint8_t *key, size_t keylen) sha1_init(&ctx->inner); sha1_update(&ctx->inner, ipad, SHA1_BLOCK_SIZE); + + memset(k, 0, sizeof(k)); + memset(ipad, 0, sizeof(ipad)); } void hmac_sha1_update(hmac_sha1_ctx *ctx, const void *data, size_t len) @@ -86,6 +89,9 @@ void hmac_sha256_init(hmac_sha256_ctx *ctx, const uint8_t *key, size_t keylen) sha256_init(&ctx->inner); sha256_update(&ctx->inner, ipad, SHA256_BLOCK_SIZE); + + memset(k, 0, sizeof(k)); + memset(ipad, 0, sizeof(ipad)); } void hmac_sha256_update(hmac_sha256_ctx *ctx, const void *data, size_t len) @@ -142,6 +148,9 @@ void hmac_sha512_init(hmac_sha512_ctx *ctx, const uint8_t *key, size_t keylen) sha512_init(&ctx->inner); sha512_update(&ctx->inner, ipad, SHA512_BLOCK_SIZE); + + memset(k, 0, sizeof(k)); + memset(ipad, 0, sizeof(ipad)); } void hmac_sha512_update(hmac_sha512_ctx *ctx, const void *data, size_t len) diff --git a/src/core/pbkdf2.c b/src/core/pbkdf2.c index 1bb7ab5..17bb6d8 100644 --- a/src/core/pbkdf2.c +++ b/src/core/pbkdf2.c @@ -37,6 +37,11 @@ static void pbkdf2_block(const uint8_t *pass, size_t passlen, } memcpy(out, t, SHA1_DIGEST_SIZE); + + memset(idx, 0, sizeof(idx)); + memset(u, 0, sizeof(u)); + memset(t, 0, sizeof(t)); + memset(&c, 0, sizeof(c)); } void pbkdf2_hmac_sha1(const uint8_t *pass, size_t passlen, @@ -59,4 +64,6 @@ void pbkdf2_hmac_sha1(const uint8_t *pass, size_t passlen, got += n; blockidx++; } + + memset(block, 0, sizeof(block)); } diff --git a/src/core/vault.c b/src/core/vault.c index 19f123e..b8e6443 100644 --- a/src/core/vault.c +++ b/src/core/vault.c @@ -15,6 +15,7 @@ #include "otp.h" #include "pbkdf2.h" #include "hmac.h" +#include "steamguard.h" /* --- on-disk header layout (byte offsets) --- */ #define OFF_MAGIC 0 @@ -174,6 +175,16 @@ static vault_result parse_payload(vault *v, const uint8_t *in, size_t len) * produce plausible-looking but wrong codes (the format doc says new * ids extend the scheme and old readers must refuse them). */ if (alg > OTP_ALG_SHA512 || type > 2) return VAULT_ERR_FORMAT; + /* digits is ignored at render time for Steam Guard (always + * STEAM_CODE_DIGITS) and otherwise must be a length AmiAuth itself + * ever writes (6-8, see uri.c/GUI edit_request) — reject anything + * else as a defense-in-depth check against a corrupted/hand-edited + * vault, consistent with the alg/type checks above. */ + if (type == 2) { + if (a.digits != STEAM_CODE_DIGITS) return VAULT_ERR_FORMAT; + } else if (a.digits < 6 || a.digits > 8) { + return VAULT_ERR_FORMAT; + } strcpy(a.type, type == 1 ? "hotp" : type == 2 ? "steam" : "totp"); strcpy(a.algorithm, otp_alg_name((otp_alg)alg)); diff --git a/src/gui/main.c b/src/gui/main.c index 7e65aab..d0e73ef 100644 --- a/src/gui/main.c +++ b/src/gui/main.c @@ -729,7 +729,7 @@ static int qr_file_request(struct Window *win, char *path, size_t cap) static int passphrase_request(const char *msg, char *buf, size_t cap) { static char stars[130]; /* mask display; kept off the stack */ - Object *win, *maskobj, *okobj, *cancelobj; + Object *win, *maskobj, *okobj, *cancelobj, *layoutobj; struct Window *w; ULONG sig = 0; size_t len = 0; @@ -758,7 +758,7 @@ static int passphrase_request(const char *msg, char *buf, size_t cap) LAYOUT_AddChild, (ULONG)okobj, LAYOUT_AddChild, (ULONG)cancelobj, TAG_END); - Object *layoutobj = NewObject(LAYOUT_GetClass(), NULL, + layoutobj = NewObject(LAYOUT_GetClass(), NULL, LAYOUT_Orientation, LAYOUT_ORIENT_VERT, LAYOUT_SpaceOuter, TRUE, LAYOUT_AddChild, (ULONG)labelobj, @@ -779,7 +779,7 @@ static int passphrase_request(const char *msg, char *buf, size_t cap) WINDOW_Layout, (ULONG)layoutobj, TAG_END); } - if (!win) return 0; + if (!win) { DisposeObject(layoutobj); return 0; } w = (struct Window *)DoMethod(win, WM_OPEN, NULL); if (!w) { DisposeObject(win); return 0; } @@ -800,6 +800,7 @@ static int passphrase_request(const char *msg, char *buf, size_t cap) else if ((r & WMHI_GADGETMASK) == PWID_CANCEL) done = 0; break; case WMHI_VANILLAKEY: + amiga_stir_keystroke(); /* per-keystroke timing entropy */ if (code == 0x0D) done = 1; /* Return */ else if (code == 0x1B) done = 0; /* Escape */ else if (code == 0x08 || code == 0x7F) { /* Backspace/Del */ @@ -1112,7 +1113,7 @@ static int uri_request(char *buf, size_t cap) WINDOW_Position, WPOS_CENTERSCREEN, WINDOW_Layout, (ULONG)layoutobj, TAG_END); - if (!win) return 0; + if (!win) { DisposeObject(layoutobj); return 0; } w = (struct Window *)DoMethod(win, WM_OPEN, NULL); if (!w) { DisposeObject(win); return 0; } @@ -1220,7 +1221,7 @@ static int edit_request(otp_account *acct) WINDOW_Position, WPOS_CENTERSCREEN, WINDOW_Layout, (ULONG)layoutobj, TAG_END); - if (!win) return 0; + if (!win) { DisposeObject(layoutobj); return 0; } w = (struct Window *)DoMethod(win, WM_OPEN, NULL); if (!w) { DisposeObject(win); return 0; } @@ -1327,7 +1328,7 @@ static int secret_meta_request(char *issuer, char *label) WINDOW_Position, WPOS_CENTERSCREEN, WINDOW_Layout, (ULONG)layoutobj, TAG_END); - if (!win) return 0; + if (!win) { DisposeObject(layoutobj); return 0; } w = (struct Window *)DoMethod(win, WM_OPEN, NULL); if (!w) { DisposeObject(win); return 0; } diff --git a/src/qr/qr.c b/src/qr/qr.c index e2a85dc..f51fdc8 100644 --- a/src/qr/qr.c +++ b/src/qr/qr.c @@ -128,10 +128,9 @@ int qr_decode_gray(const unsigned char *gray, int w, int h, { int result; - if (cap > 0) - uri[0] = '\0'; if (!gray || !uri || cap == 0 || w <= 0 || h <= 0) return QR_ERR_ARGS; + uri[0] = '\0'; result = decode_once(gray, w, h, uri, cap); if (result == QR_ERR_NOCODE)