sign_psbt: fix reply count for exact chunk sizes
What the developer wrote
Authored by kkdao
sign_psbt: fix reply count for exact chunk sizes
When the signed PSBT length is an exact multiple of the output chunk
size, the reply advertised one message too many. The host then asked
for the extra chunk and the device aborted on an assertion. Count the
messages by rounding up instead.
What changed, and why it matters
This commit fixes a bug in how Blockstream Jade counts the number of message chunks needed to send back a signed Bitcoin transaction (PSBT). When the transaction size was an exact multiple of the chunk size, the device told the host there was one extra chunk. When the host asked for that non-existent chunk, the device hit an internal assertion and aborted. The fix rounds the chunk count up correctly. A new test exercises sizes right at the chunk-size boundary.
Treat as a low-severity reliability/denial-of-service fix. Ensure the patch is included in firmware builds and that the new boundary tests pass. No immediate incident response is warranted unless the crash can be chained with other bugs; the commit itself does not indicate that.
Security signals we found
Integer division off-by-one in chunk/message count
Device abort on assertion when host requests non-existent chunk
Denial-of-service-like symptom: valid signing flow can crash the device
Boundary-size test cases added for exact multiples of chunk size
Evidence from the diff
In sign_psbt_process(), the number of output messages was computed as (bytes_len / PSBT_OUT_CHUNK_SIZE) + 1, which overcounts by one when bytes_len is evenly divisible by PSBT_OUT_CHUNK_SIZE. The corrected expression ((bytes_len - 1) / PSBT_OUT_CHUNK_SIZE) + 1 gives the proper ceiling division. The patch also adds a guard ensuring bytes_len is non-zero before computing the count. A Python test helper pads a PSBT with a proprietary unknown field and verifies signing at sizes 3007/3008/3009 and 6015/6016/6017 bytes, covering one and two chunk boundaries.
Changed components
main/process/sign_psbt.ctests/rpc/test_sign_psbt.pyInspect captured patch +38 / −2
### main/process/sign_psbt.c
@@ -1253,7 +1253,7 @@ void sign_psbt_process(void* process_ptr)
// Serialise signed psbt
uint8_t* bytes = NULL;
size_t bytes_len = 0;
- if (!serialise_psbt(psbt, &bytes, &bytes_len)) {
+ if (!serialise_psbt(psbt, &bytes, &bytes_len) || !bytes_len) {
jade_process_reject_message(process, CBOR_RPC_INTERNAL_ERROR, "Failed to serialise sign psbt");
goto cleanup;
}
@@ -1264,7 +1264,7 @@ void sign_psbt_process(void* process_ptr)
size_t original_id_len = 0;
rpc_get_id(&process->ctx.value, original_id, sizeof(original_id), &original_id_len);
- const int nmsgs = (bytes_len / PSBT_OUT_CHUNK_SIZE) + 1;
+ const int nmsgs = ((bytes_len - 1) / PSBT_OUT_CHUNK_SIZE) + 1;
uint8_t* const msgbuf = JADE_MALLOC(MAX_OUTPUT_MSG_SIZE);
uint8_t* chunk = bytes;
for (size_t imsg = 0; imsg < nmsgs; ++imsg) {
### tests/rpc/test_sign_psbt.py
@@ -68,3 +68,39 @@ def test_sign_ss_psbt(jade, mnemonic, test_case):
@with_test_cases('tests/rpc/data/sign_psbt/pset_ss_*.json')
def test_sign_ss_pset(jade, mnemonic, test_case):
_test_sign_psbt(jade, test_case)
+
+
+def _psbt_with_padding(psbt_bin, padding_len):
+ '''Add a proprietary input field (key 0xfc, id "jade", subtype 0)
+ to pad the psbt size without side effects'''
+ psbt = wally.psbt_from_bytes(psbt_bin, 0)
+ assert wally.psbt_get_input_unknowns_size(psbt, 0) == 0
+ unknowns = wally.map_init(1, None)
+ wally.map_add(unknowns, b'\xfc\x04jade\x00', bytes(padding_len))
+ wally.psbt_set_input_unknowns(psbt, 0, unknowns)
+ return wally.psbt_to_bytes(psbt, 0)
+
+
+@pytest.mark.skipif(transport_is_not('libjade'), reason='libjade only')
+@pytest.mark.mnemonic(mnemonics.singlesig)
+@with_test_cases('tests/rpc/data/sign_psbt/psbt_ss_p2tr_default_all.json')
+def test_sign_psbt_chunk_boundaries(jade, mnemonic, test_case):
+ # PSBT_OUT_CHUNK_SIZE is 3072 - 64 = 3008 in standard builds, including
+ # libjade. Test +/-1 of one and two chunks plus exact multiples.
+ for output_size in [3007, 3008, 3009, 6015, 6016, 6017]:
+ signed_psbt = test_case['expected_output']['psbt']
+ padding_len = output_size - len(signed_psbt)
+ # Account for the unknown key and CompactSize-encoded key/value lengths.
+ padding_len -= len(_psbt_with_padding(signed_psbt, padding_len)) - output_size
+ expected_psbt = _psbt_with_padding(signed_psbt, padding_len)
+ assert len(expected_psbt) == output_size
+
+ padded_test_case = {
+ **test_case,
+ 'input': {
+ **test_case['input'],
+ 'psbt': _psbt_with_padding(test_case['input']['psbt'], padding_len),
+ },
+ 'expected_output': {**test_case['expected_output'], 'psbt': expected_psbt},
+ }
+ _test_sign_psbt(jade, padded_test_case)Why this scored 43/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.