Public commit record
82/100 · Strong
What the developer wrote
Authored by M1nd3r
fix(crypto): fix cash_encode
- Fixed description of buffer sizes in the header file.
- Added check of hrp length in cash_encode. Otherwise it allows to encode addresses that cannot be decoded using cash_decode.
- Adjusted naming of constants and parameters for clarity and consistency.
[no changelog]
✓ Descriptive subject✓ Uses a recognizable type or scope✓ Provides detailed explanatory context✓ Explains rationale or failure mode
What changed, and why it matters
This commit fixes a mismatch in Trezor's Bitcoin Cash address encoding. Previously, the encoder could create addresses with an overly long human-readable prefix (the part before the colon, like 'bitcoincash') that the decoder would later reject as invalid. The patch adds a length check so the encoder refuses to produce addresses that cannot be decoded. It also renames variables and updates comments for clarity, with no functional changes beyond the new length check.
Recommended action
Review callers of cash_addr_encode/cash_encode to confirm they handle the new failure mode (return 0) correctly, and verify that downstream UI or transaction code does not assume encoding always succeeds. Consider adding unit tests for HRP length edge cases.
Security signals we found
01
Added input-validation boundary check on hrp length in cash_encode
02
Fixed encode/decode asymmetry that allowed generation of invalid cashaddr strings
03
Header comments corrected to reflect actual required output buffer sizes
04
No changelog entry despite functional behavior change
Evidence from the diff
The change is in crypto/cash_addr.c/h. The core fix is in cash_encode(): it now returns 0 if the HRP exceeds MAX_HRP_LEN (20) during encoding. Without this check, cash_encode() could emit a cashaddr string whose HRP is longer than MAX_HRP_LEN, which cash_decode() would reject because it enforces hrp_len <= MAX_HRP_LEN. This created an encode/decode asymmetry. The rest of the diff is renaming (data_len -> data_size, MAX_DATA_SIZE -> MAX_PROG_SIZE, etc.) and header comment corrections for buffer-size descriptions.
Changed components
crypto/cash_addr.ccrypto/cash_addr.hBitcoin Cash address encoding/decoding functions (cash_encode, cash_decode, cash_addr_encode, cash_addr_decode)Inspect captured patch +65 / −58
diff --git a/crypto/cash_addr.c b/crypto/cash_addr.c
index 24888445..1057e7ef 100644
--- a/crypto/cash_addr.c
+++ b/crypto/cash_addr.c
@@ -25,10 +25,10 @@
#include "cash_addr.h"
-#define MAX_CASHADDR_SIZE 129
+#define MAX_CASHADDR_LEN 129
#define MAX_BASE32_SIZE 104
-#define MAX_DATA_SIZE 65
-#define MAX_HRP_SIZE 20
+#define MAX_PROG_SIZE 65
+#define MAX_HRP_LEN 20
#define CHECKSUM_SIZE 8
uint64_t cashaddr_polymod_step(uint64_t pre) {
@@ -52,7 +52,7 @@ static const int8_t charset_rev[128] = {
3, 16, 11, 28, 12, 14, 6, 4, 2, -1, -1, -1, -1, -1};
int cash_encode(char* output, const char* hrp, const uint8_t* data,
- size_t data_len) {
+ size_t data_size) {
uint64_t chk = 1;
size_t i = 0;
while (hrp[i] != 0) {
@@ -63,13 +63,16 @@ int cash_encode(char* output, const char* hrp, const uint8_t* data,
*(output++) = ch;
chk = cashaddr_polymod_step(chk) ^ (ch & 0x1f);
++i;
+ if (i > MAX_HRP_LEN) {
+ return 0;
+ }
}
- if (i + 1 + data_len + CHECKSUM_SIZE > MAX_CASHADDR_SIZE) {
+ if (i + 1 + data_size + CHECKSUM_SIZE > MAX_CASHADDR_LEN) {
return 0;
}
chk = cashaddr_polymod_step(chk);
*(output++) = ':';
- for (i = 0; i < data_len; ++i) {
+ for (i = 0; i < data_size; ++i) {
if (*data >> 5) return 0;
chk = cashaddr_polymod_step(chk) ^ (*data);
*(output++) = charset[*(data++)];
@@ -85,27 +88,28 @@ int cash_encode(char* output, const char* hrp, const uint8_t* data,
return 1;
}
-int cash_decode(char* hrp, uint8_t* data, size_t* data_len, const char* input) {
+int cash_decode(char* hrp, uint8_t* data, size_t* data_size,
+ const char* input) {
uint64_t chk = 1;
size_t i = 0;
size_t input_len = strlen(input);
size_t hrp_len = 0;
int have_lower = 0, have_upper = 0;
- if (input_len < CHECKSUM_SIZE || input_len > MAX_CASHADDR_SIZE) {
+ if (input_len < CHECKSUM_SIZE || input_len > MAX_CASHADDR_LEN) {
return 0;
}
- *data_len = 0;
- while (*data_len < input_len && input[(input_len - 1) - *data_len] != ':') {
- ++(*data_len);
+ *data_size = 0;
+ while (*data_size < input_len && input[(input_len - 1) - *data_size] != ':') {
+ ++(*data_size);
}
- hrp_len = input_len - (1 + *data_len);
- if (1 + *data_len >= input_len || hrp_len > MAX_HRP_SIZE ||
- *data_len < CHECKSUM_SIZE ||
- *data_len > CHECKSUM_SIZE + MAX_BASE32_SIZE) {
+ hrp_len = input_len - (1 + *data_size);
+ if (1 + *data_size >= input_len || hrp_len > MAX_HRP_LEN ||
+ *data_size < CHECKSUM_SIZE ||
+ *data_size > CHECKSUM_SIZE + MAX_BASE32_SIZE) {
return 0;
}
// subtract checksum
- *(data_len) -= CHECKSUM_SIZE;
+ *(data_size) -= CHECKSUM_SIZE;
for (i = 0; i < hrp_len; ++i) {
int ch = input[i];
if (ch < 33 || ch > 126) {
@@ -165,25 +169,25 @@ static int convert_bits(uint8_t* out, size_t* outlen, int outbits,
return 1;
}
-int cash_addr_encode(char* output, const char* hrp, const uint8_t* data,
- size_t data_len) {
+int cash_addr_encode(char* output, const char* hrp, const uint8_t* prog,
+ size_t prog_size) {
uint8_t base32[MAX_BASE32_SIZE] = {0};
- size_t base32len = 0;
- if (data_len < 2 || data_len > MAX_DATA_SIZE) return 0;
- convert_bits(base32, &base32len, 5, data, data_len, 8, 1);
- return cash_encode(output, hrp, base32, base32len);
+ size_t base32size = 0;
+ if (prog_size < 2 || prog_size > MAX_PROG_SIZE) return 0;
+ convert_bits(base32, &base32size, 5, prog, prog_size, 8, 1);
+ return cash_encode(output, hrp, base32, base32size);
}
-int cash_addr_decode(uint8_t* witdata, size_t* witdata_len, const char* hrp,
+int cash_addr_decode(uint8_t* prog, size_t* prog_size, const char* hrp,
const char* addr) {
uint8_t data[MAX_BASE32_SIZE] = {0};
- char hrp_actual[MAX_HRP_SIZE + 1] = {0};
- size_t data_len = 0;
- if (!cash_decode(hrp_actual, data, &data_len, addr)) return 0;
- if (data_len == 0 || data_len > MAX_BASE32_SIZE) return 0;
- if (strncmp(hrp, hrp_actual, MAX_HRP_SIZE + 1) != 0) return 0;
- *witdata_len = 0;
- if (!convert_bits(witdata, witdata_len, 8, data, data_len, 5, 0)) return 0;
- if (*witdata_len < 2 || *witdata_len > MAX_DATA_SIZE) return 0;
+ char hrp_actual[MAX_HRP_LEN + 1] = {0};
+ size_t data_size = 0;
+ if (!cash_decode(hrp_actual, data, &data_size, addr)) return 0;
+ if (data_size == 0 || data_size > MAX_BASE32_SIZE) return 0;
+ if (strncmp(hrp, hrp_actual, MAX_HRP_LEN + 1) != 0) return 0;
+ *prog_size = 0;
+ if (!convert_bits(prog, prog_size, 8, data, data_size, 5, 0)) return 0;
+ if (*prog_size < 2 || *prog_size > MAX_PROG_SIZE) return 0;
return 1;
}
diff --git a/crypto/cash_addr.h b/crypto/cash_addr.h
index fd7dd44f..5f27edb8 100644
--- a/crypto/cash_addr.h
+++ b/crypto/cash_addr.h
@@ -26,52 +26,55 @@
/** Encode a Cashaddr address
*
- * Out: output: Pointer to a buffer of size 105 + strlen(hrp) that will be
- * updated to contain the null-terminated address.
- * In: hrp: Pointer to the null-terminated human readable part to use
+ * Out: output: Pointer to a buffer of size min(114 + strlen(hrp), 130)
+ * that will be updated to contain the null-terminated
+ * address.
+ * In: hrp: Pointer to the null-terminated human readable part to use
* (chain/network specific).
- * prog: Data bytes for the hash (between 21 and 65 bytes).
- * prog_len: Number of data bytes in prog.
+ * prog: Data bytes for the hash (between 21 and 65 bytes).
+ * prog_size: Number of data bytes in prog.
* Returns 1 if successful.
*/
int cash_addr_encode(char *output, const char *hrp, const uint8_t *prog,
- size_t prog_len);
+ size_t prog_size);
/** Decode a CashAddr address
*
- * Out: prog: Pointer to a buffer of size 65 that will be updated to
- * contain the witness program bytes.
- * prog_len: Pointer to a size_t that will be updated to contain the
- * length of bytes in prog. hrp: Pointer to the null-terminated human
- * readable part that is expected (chain/network specific). addr: Pointer to
- * the null-terminated address. Returns 1 if successful.
+ * Out: prog: Pointer to a buffer of size 65 that will be updated to
+ * contain the witness program bytes.
+ * prog_size: Pointer to a size_t that will be updated to contain the
+ * length of bytes in prog.
+ * In: hrp: Pointer to the null-terminated human readable part that is
+ * expected (chain/network specific).
+ * addr: Pointer to the null-terminated address.
+ * Returns 1 if successful.
*/
-int cash_addr_decode(uint8_t *prog, size_t *prog_len, const char *hrp,
+int cash_addr_decode(uint8_t *prog, size_t *prog_size, const char *hrp,
const char *addr);
/** Encode a Cash string
*
- * Out: output: Pointer to a buffer of size strlen(hrp) + data_len + 8 that
- * will be updated to contain the null-terminated Cash string.
- * In: hrp : Pointer to the null-terminated human readable part.
- * data : Pointer to an array of 5-bit values.
- * data_len: Length of the data array.
+ * Out: output: Pointer to a buffer of size strlen(hrp) + data_size + 10 that
+ * will be updated to contain the null-terminated Cash string.
+ * In: hrp: Pointer to the null-terminated human readable part.
+ * data: Pointer to an array of 5-bit values.
+ * data_size: Length of the data array.
* Returns 1 if successful.
*/
int cash_encode(char *output, const char *hrp, const uint8_t *data,
- size_t data_len);
+ size_t data_size);
/** Decode a Cash string
*
- * Out: hrp: Pointer to a buffer of size strlen(input) - 6. Will be
- * updated to contain the null-terminated human readable part.
- * data: Pointer to a buffer of size strlen(input) - 8 that will
- * hold the encoded 5-bit data values.
- * data_len: Pointer to a size_t that will be updated to be the number
- * of entries in data.
- * In: input: Pointer to a null-terminated Cash string.
+ * Out: hrp: Pointer to a buffer of size strlen(input) - 6. Will be
+ * updated to contain the null-terminated human readable part.
+ * data: Pointer to a buffer of size strlen(input) - 8 that will
+ * hold the encoded 5-bit data values.
+ * data_size: Pointer to a size_t that will be updated to be the number
+ * of entries in data.
+ * In: input: Pointer to a null-terminated Cash string.
* Returns 1 if succesful.
*/
-int cash_decode(char *hrp, uint8_t *data, size_t *data_len, const char *input);
+int cash_decode(char *hrp, uint8_t *data, size_t *data_size, const char *input);
#endif
Risk score
Our methodology →Why this scored 37/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.