diff --git a/app/src/crypto.c b/app/src/crypto.c index e18abd8..4bf3e6b 100644 --- a/app/src/crypto.c +++ b/app/src/crypto.c @@ -411,7 +411,7 @@ zxerr_t crypto_sign(const parser_tx_t *txObj, uint8_t *output, uint16_t outputLe signature_section.hashes.hashesLen += 2; // Include Masp hash in the signature if it's there - if (txObj->transaction.isMasp) { + if ((txObj->typeTx == Transfer && txObj->transfer.has_shielded_hash) || (txObj->typeTx == IBC && txObj->ibc.transfer.has_shielded_hash)) { #if !defined(APP_TESTING) if (get_state() != STATE_EXTRACT_SPENDS) { return zxerr_unknown; diff --git a/app/src/parser_impl.c b/app/src/parser_impl.c index dffe807..776e3cb 100644 --- a/app/src/parser_impl.c +++ b/app/src/parser_impl.c @@ -27,9 +27,7 @@ parser_error_t _read(parser_context_t *ctx, parser_tx_t *v) { CHECK_ERROR(validateTransactionParams(v)) - if(ctx->tx_obj->transaction.isMasp || ctx->tx_obj->ibc.is_ibc) { - CHECK_ERROR(verifyShieldedHash(ctx)) - } + CHECK_ERROR(verifyShieldedHash(ctx)) if (ctx->offset != ctx->bufferLen) { return parser_unexpected_unparsed_bytes; @@ -51,7 +49,7 @@ parser_error_t getNumItems(const parser_context_t *ctx, uint8_t *numItems) { break; case Transfer: - if(ctx->tx_obj->transaction.isMasp) { + if(ctx->tx_obj->transfer.has_shielded_hash) { uint8_t items = 1; items += 3 * ctx->tx_obj->transaction.sections.maspBuilder.builder.sapling_builder.n_outputs; // print from outputs items += 3 * ctx->tx_obj->transaction.sections.maspBuilder.builder.sapling_builder.n_spends; // print from spends @@ -130,7 +128,7 @@ parser_error_t getNumItems(const parser_context_t *ctx, uint8_t *numItems) { case IBC: *numItems = (app_mode_expert() ? IBC_EXPERT_PARAMS : IBC_NORMAL_PARAMS); - if(ctx->tx_obj->transaction.isMasp) { + if(ctx->tx_obj->ibc.transfer.has_shielded_hash) { *numItems += 3 * ctx->tx_obj->transaction.sections.maspBuilder.builder.sapling_builder.n_outputs; // print from outputs *numItems += 3 * ctx->tx_obj->transaction.sections.maspBuilder.builder.sapling_builder.n_spends; // print from spends } diff --git a/app/src/parser_impl_txn.c b/app/src/parser_impl_txn.c index c46fe77..0f5afe8 100644 --- a/app/src/parser_impl_txn.c +++ b/app/src/parser_impl_txn.c @@ -1255,7 +1255,8 @@ parser_error_t readSections(parser_context_t *ctx, parser_tx_t *v) { if (v->transaction.sections.sectionLen > 7) { return parser_invalid_output_buffer; } - v->transaction.isMasp = false; + v->transaction.hasMaspTx = false; + v->transaction.hasMaspBuilder = false; v->transaction.sections.extraDataLen = 0; v->transaction.sections.signaturesLen = 0; @@ -1296,11 +1297,12 @@ parser_error_t readSections(parser_context_t *ctx, parser_tx_t *v) { #if defined(COMPILE_MASP) case DISCRIMINANT_MASP_TX: // Identify tx has masp tx - v->transaction.isMasp = true; + v->transaction.hasMaspTx = true; CHECK_ERROR(readMaspTx(ctx, &v->transaction.sections.maspTx)) v->transaction.maspTx_idx = i+1; break; case DISCRIMINANT_MASP_BUILDER: + v->transaction.hasMaspBuilder = true; CHECK_ERROR(readMaspBuilder(ctx, &v->transaction.sections.maspBuilder)) break; #endif @@ -1421,24 +1423,36 @@ parser_error_t verifyShieldedHash(parser_context_t *ctx) { } #if defined(LEDGER_SPECIFIC) - // compute tx_id hash uint8_t tx_id_hash[HASH_LEN] = {0}; - if (tx_hash_txId(ctx->tx_obj, tx_id_hash) != zxerr_ok) { - return parser_unexpected_error; - } - - if (ctx->tx_obj->transaction.sections.maspBuilder.target_hash.len == HASH_LEN) { - if (memcmp(tx_id_hash, ctx->tx_obj->transaction.sections.maspBuilder.target_hash.ptr, HASH_LEN) != 0) { + if (ctx->tx_obj->transaction.hasMaspTx) { + if (!ctx->tx_obj->transaction.hasMaspBuilder) { + // MASP Trabsactions must be accompanied by MASP Builders + return parser_unexpected_error; + } else if (tx_hash_txId(ctx->tx_obj, tx_id_hash) != zxerr_ok) { + // compute tx_id hash + return parser_unexpected_error; + } else if (memcmp(tx_id_hash, ctx->tx_obj->transaction.sections.maspBuilder.target_hash.ptr, HASH_LEN) != 0) { + // MASP Builder must have the correct TxId return parser_invalid_target_hash; } } - if (ctx->tx_obj->transfer.has_shielded_hash && memcmp(ctx->tx_obj->transfer.shielded_hash.ptr, tx_id_hash, HASH_LEN) != 0) { - return parser_invalid_target_hash; - } - - if(ctx->tx_obj->ibc.transfer.has_shielded_hash && memcmp(ctx->tx_obj->ibc.transfer.shielded_hash.ptr, tx_id_hash, HASH_LEN) != 0) { - return parser_invalid_target_hash; + if (ctx->tx_obj->typeTx == Transfer && ctx->tx_obj->transfer.has_shielded_hash) { + if (!ctx->tx_obj->transaction.hasMaspTx) { + // MASP transfers must be accompanied by MASP Transactions + return parser_unexpected_error; + } else if (memcmp(ctx->tx_obj->transfer.shielded_hash.ptr, tx_id_hash, HASH_LEN) != 0) { + // MASP Transactions must have the correct TxId + return parser_invalid_target_hash; + } + } else if (ctx->tx_obj->typeTx == IBC && ctx->tx_obj->ibc.transfer.has_shielded_hash) { + if (!ctx->tx_obj->transaction.hasMaspTx) { + // IBC MASP transfers must be accompanied by MASP Transactions + return parser_unexpected_error; + } else if (memcmp(ctx->tx_obj->ibc.transfer.shielded_hash.ptr, tx_id_hash, HASH_LEN) != 0) { + // MASP Transactions must have the correct TxId + return parser_invalid_target_hash; + } } #endif diff --git a/app/src/parser_print_txn.c b/app/src/parser_print_txn.c index 73e4d4a..a97b23d 100644 --- a/app/src/parser_print_txn.c +++ b/app/src/parser_print_txn.c @@ -174,8 +174,8 @@ static parser_error_t printTransferTxn( const parser_context_t *ctx, // Get pointer to the outputs bytes_t out = ctx->tx_obj->transaction.sections.maspBuilder.builder.sapling_builder.outputs; // Compute number of spends/outs in the builder tx , and number of itemns to be printer for each - uint32_t n_spends = ctx->tx_obj->transaction.sections.maspBuilder.builder.sapling_builder.n_spends * (uint32_t) ctx->tx_obj->transaction.isMasp; - uint32_t n_outs = ctx->tx_obj->transaction.sections.maspBuilder.builder.sapling_builder.n_outputs * (uint32_t) ctx->tx_obj->transaction.isMasp; + uint32_t n_spends = ctx->tx_obj->transaction.sections.maspBuilder.builder.sapling_builder.n_spends * (uint32_t) ctx->tx_obj->transfer.has_shielded_hash; + uint32_t n_outs = ctx->tx_obj->transaction.sections.maspBuilder.builder.sapling_builder.n_outputs * (uint32_t) ctx->tx_obj->transfer.has_shielded_hash; uint16_t spend_index = 0; uint16_t out_index = 0; @@ -1132,8 +1132,8 @@ static parser_error_t printIBCTxn( const parser_context_t *ctx, // Get pointer to the outputs bytes_t out = ctx->tx_obj->transaction.sections.maspBuilder.builder.sapling_builder.outputs; // Compute number of spends/outs in the builder tx , and number of itemns to be printer for each - uint32_t n_spends = ctx->tx_obj->transaction.sections.maspBuilder.builder.sapling_builder.n_spends * (uint32_t) ctx->tx_obj->transaction.isMasp; - uint32_t n_outs = ctx->tx_obj->transaction.sections.maspBuilder.builder.sapling_builder.n_outputs * (uint32_t) ctx->tx_obj->transaction.isMasp; + uint32_t n_spends = ctx->tx_obj->transaction.sections.maspBuilder.builder.sapling_builder.n_spends * (uint32_t) ctx->tx_obj->ibc.transfer.has_shielded_hash; + uint32_t n_outs = ctx->tx_obj->transaction.sections.maspBuilder.builder.sapling_builder.n_outputs * (uint32_t) ctx->tx_obj->ibc.transfer.has_shielded_hash; uint16_t spend_index = 0; uint16_t out_index = 0; @@ -1445,8 +1445,8 @@ static parser_error_t printNFTIBCTxn( const parser_context_t *ctx, // Get pointer to the outputs bytes_t out = ctx->tx_obj->transaction.sections.maspBuilder.builder.sapling_builder.outputs; // Compute number of spends/outs in the builder tx , and number of itemns to be printer for each - uint32_t n_spends = ctx->tx_obj->transaction.sections.maspBuilder.builder.sapling_builder.n_spends * (uint32_t) ctx->tx_obj->transaction.isMasp; - uint32_t n_outs = ctx->tx_obj->transaction.sections.maspBuilder.builder.sapling_builder.n_outputs * (uint32_t) ctx->tx_obj->transaction.isMasp; + uint32_t n_spends = ctx->tx_obj->transaction.sections.maspBuilder.builder.sapling_builder.n_spends * (uint32_t) ctx->tx_obj->ibc.transfer.has_shielded_hash; + uint32_t n_outs = ctx->tx_obj->transaction.sections.maspBuilder.builder.sapling_builder.n_outputs * (uint32_t) ctx->tx_obj->ibc.transfer.has_shielded_hash; uint16_t spend_index = 0; uint16_t out_index = 0; diff --git a/app/src/parser_txdef.h b/app/src/parser_txdef.h index 101429f..c7648e9 100644 --- a/app/src/parser_txdef.h +++ b/app/src/parser_txdef.h @@ -280,7 +280,8 @@ typedef struct { header_t header; sections_t sections; uint8_t maspTx_idx; - bool isMasp; + bool hasMaspTx; + bool hasMaspBuilder; } transaction_t; diff --git a/tests/testvectors.json b/tests/testvectors.json index fe21cd2..f4f10d0 100644 --- a/tests/testvectors.json +++ b/tests/testvectors.json @@ -50947,4 +50947,4 @@ ], "valid": true } -] \ No newline at end of file +]