Using generic and simpler format_sats_amount() for swap as well
What the developer wrote
Authored by Ilya Artemov
Using generic and simpler format_sats_amount() for swap as well
What changed, and why it matters
This commit replaces a custom Bitcoin amount-formatting routine used during Ledger's cryptocurrency swap feature with a shared, simpler utility. The old code converted an 8-byte big-endian amount into a human-readable decimal string with a coin ticker. The new code uses the project's generic `format_sats_amount()` function. The change removes about 120 lines of low-level bit-manipulation code. On its own, the commit does not claim to fix a security bug, and the diff does not show an obvious vulnerability. However, any change to amount display logic in a swap context is security-sensitive because a bug could mislead a user about how much value is being exchanged.
Review the implementation of `format_sats_amount()` and `read_u64_be()` to confirm they correctly handle 8-byte big-endian amounts, enforce buffer sizes for `params->printable_amount`, and behave identically to the removed routine for all valid inputs. Because this code is used during swap transactions, any discrepancy in displayed amounts could have financial impact. Consider adding unit tests covering edge amounts (zero, max uint64, leading zeros, smallest subunit).
Security signals we found
Amount formatting code changed in swap context
Custom BCD/double-dabble conversion removed in favor of generic helper
No explicit security claim in commit title or message
No input-length or overflow checks visible in diff
Cast comment suggests compiler-specific workaround, not a semantic fix
Evidence from the diff
The deleted btchip_convert_hex_amount_to_displayable_no_globals() performed a binary-coded-decimal-like conversion: it shifted an 8-byte amount bit-by-bit into a scratch buffer, applied the classic “add 3 if >= 5” double-dabble rule, then emitted leading digits, a decimal point, and up to 8 fractional digits. The replacement calls format_sats_amount(COIN_COINID_SHORT, read_u64_be(amount, 0), params->printable_amount). The cast to uint64_t is noted by the author as working around a compiler issue. The diff is purely a refactor; no bounds checks, input validation, or error handling are visibly added or removed. The security relevance depends on whether format_sats_amount() is at least as correct and safe as the removed routine.
Changed components
src/swap/handle_get_printable_amount.csrc/swap/btchip_bcd.csrc/swap/btchip_bcd.hLedger Bitcoin app swap amount displayInspect captured patch +5 / −120
diff --git a/src/swap/btchip_bcd.c b/src/swap/btchip_bcd.c
deleted file mode 100644
index 20e9938..0000000
--- a/src/swap/btchip_bcd.c
+++ /dev/null
@@ -1,88 +0,0 @@
-/*******************************************************************************
- * Ledger App - Bitcoin Wallet
- * (c) 2016-2019 Ledger
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- ********************************************************************************/
-
-#define SCRATCH_SIZE 21
-
-unsigned char btchip_convert_hex_amount_to_displayable_no_globals(unsigned char* amount,
- unsigned char* out) {
- unsigned char LOOP1 = 13;
- unsigned char LOOP2 = 8;
-
- unsigned short scratch[SCRATCH_SIZE];
- unsigned char offset = 0;
- unsigned char nonZero = 0;
- unsigned char i;
- unsigned char targetOffset = 0;
- unsigned char workOffset;
- unsigned char j;
- unsigned char nscratch = SCRATCH_SIZE;
- unsigned char smin = nscratch - 2;
- unsigned char comma = 0;
-
- for (i = 0; i < SCRATCH_SIZE; i++) {
- scratch[i] = 0;
- }
- for (i = 0; i < 8; i++) {
- for (j = 0; j < 8; j++) {
- unsigned char k;
- unsigned short shifted_in =
- (((amount[i] & 0xff) & ((1 << (7 - j)))) != 0) ? (short) 1 : (short) 0;
- for (k = smin; k < nscratch; k++) {
- scratch[k] += ((scratch[k] >= 5) ? 3 : 0);
- }
- if (scratch[smin] >= 8) {
- smin -= 1;
- }
- for (k = smin; k < nscratch - 1; k++) {
- scratch[k] = ((scratch[k] << 1) & 0xF) | ((scratch[k + 1] >= 8) ? 1 : 0);
- }
- scratch[nscratch - 1] =
- ((scratch[nscratch - 1] << 1) & 0x0F) | (shifted_in == 1 ? 1 : 0);
- }
- }
-
- for (i = 0; i < LOOP1; i++) {
- if (!nonZero && (scratch[offset] == 0)) {
- offset++;
- } else {
- nonZero = 1;
- out[targetOffset++] = scratch[offset++] + '0';
- }
- }
- if (targetOffset == 0) {
- out[targetOffset++] = '0';
- }
- workOffset = offset;
- for (i = 0; i < LOOP2; i++) {
- unsigned char allZero = 1;
- for (j = i; j < LOOP2; j++) {
- if (scratch[workOffset + j] != 0) {
- allZero = 0;
- break;
- }
- }
- if (allZero) {
- break;
- }
- if (!comma) {
- out[targetOffset++] = '.';
- comma = 1;
- }
- out[targetOffset++] = scratch[offset++] + '0';
- }
- return targetOffset;
-}
diff --git a/src/swap/btchip_bcd.h b/src/swap/btchip_bcd.h
deleted file mode 100644
index 5a72ce0..0000000
--- a/src/swap/btchip_bcd.h
+++ /dev/null
@@ -1,21 +0,0 @@
-/*******************************************************************************
- * Ledger App - Bitcoin Wallet
- * (c) 2016-2019 Ledger
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- ********************************************************************************/
-
-#pragma once
-
-unsigned char btchip_convert_hex_amount_to_displayable_no_globals(unsigned char *amount,
- unsigned char *out);
diff --git a/src/swap/handle_get_printable_amount.c b/src/swap/handle_get_printable_amount.c
index c92967d..f77a1c7 100644
--- a/src/swap/handle_get_printable_amount.c
+++ b/src/swap/handle_get_printable_amount.c
@@ -1,10 +1,8 @@
-#include <string.h>
-#include <stdint.h>
+#include "read.h"
+#include "display_utils.h"
#include "handle_get_printable_amount.h"
-#include "btchip_bcd.h"
-
#define MAX_NON_PRINTABLE_AMOUNT_LEN 8
int handle_get_printable_amount(get_printable_amount_parameters_t *params) {
@@ -18,13 +16,9 @@ int handle_get_printable_amount(get_printable_amount_parameters_t *params) {
memcpy(amount + (MAX_NON_PRINTABLE_AMOUNT_LEN - params->amount_length),
params->amount,
params->amount_length);
- int res_length =
- btchip_convert_hex_amount_to_displayable_no_globals(amount,
- (uint8_t *) params->printable_amount);
- params->printable_amount[res_length] = ' ';
- size_t coin_name_length = strlen(COIN_COINID_SHORT);
- memmove(¶ms->printable_amount[res_length + 1], COIN_COINID_SHORT, coin_name_length);
- params->printable_amount[res_length + coin_name_length + 1] = '\0';
+ format_sats_amount(COIN_COINID_SHORT,
+ (uint64_t) (read_u64_be(amount, 0)), // Cast prevents weird compilo bug
+ params->printable_amount);
return 1;
}
Why this scored 26/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.