diff --git a/.github/workflows/build_and_functional_tests.yml b/.github/workflows/build_and_functional_tests.yml index cb6844b..696f27c 100644 --- a/.github/workflows/build_and_functional_tests.yml +++ b/.github/workflows/build_and_functional_tests.yml @@ -5,11 +5,21 @@ name: Build and run functional tests using ragger through reusable workflow # resulting binaries. # It then calls another reusable workflow to run the Ragger tests on the compiled application binary. # -# While this workflow is optional, having functional testing on your application is mandatory and this workflow and +# The build part of this workflow is mandatory, this ensures that the app will be deployable in the Ledger App Store. +# While the test part of this workflow is optional, having functional testing on your application is mandatory and this workflow and # tooling environment is meant to be easy to use and adapt after forking your application on: workflow_dispatch: + inputs: + golden_run: + type: choice + required: true + default: 'Raise an error (default)' + description: CI behavior if the test snapshots are different than expected. + options: + - 'Raise an error (default)' + - 'Open a PR' push: branches: - master @@ -30,5 +40,4 @@ jobs: uses: LedgerHQ/ledger-app-workflows/.github/workflows/reusable_ragger_tests.yml@v1 with: download_app_binaries_artifact: "compiled_app_binaries" - test_dir: tests - run_for_devices: '["nanos", "nanox", "nanosp", "stax"]' + regenerate_snapshots: ${{ inputs.golden_run == 'Open a PR' }} diff --git a/.github/workflows/codeql_checks.yml b/.github/workflows/codeql_checks.yml index e18180b..e6b3e51 100644 --- a/.github/workflows/codeql_checks.yml +++ b/.github/workflows/codeql_checks.yml @@ -17,24 +17,21 @@ jobs: analyse: name: Analyse strategy: + fail-fast: false matrix: - sdk: [ "$NANOS_SDK", "$NANOX_SDK", "$NANOSP_SDK" ] - #'cpp' covers C and C++ - language: [ 'cpp' ] + sdk: ["$NANOX_SDK", "$NANOSP_SDK", "$STAX_SDK", "$FLEX_SDK"] + # 'cpp' covers C and C++ + language: ['cpp'] runs-on: ubuntu-latest - permissions: - actions: read - contents: read - security-events: write container: - image: ghcr.io/ledgerhq/ledger-app-builder/ledger-app-builder-legacy:latest + image: ghcr.io/ledgerhq/ledger-app-builder/ledger-app-builder-lite:latest steps: - name: Clone - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Initialize CodeQL - uses: github/codeql-action/init@v2 + uses: github/codeql-action/init@v3 with: languages: ${{ matrix.language }} queries: security-and-quality @@ -45,4 +42,4 @@ jobs: make BOLOS_SDK=${{ matrix.sdk }} - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v2 + uses: github/codeql-action/analyze@v3 diff --git a/.github/workflows/coding_style_checks.yml b/.github/workflows/coding_style_checks.yml index 6be2786..533fc38 100644 --- a/.github/workflows/coding_style_checks.yml +++ b/.github/workflows/coding_style_checks.yml @@ -22,4 +22,3 @@ jobs: with: source: './src' extensions: 'h,c' - version: 11 diff --git a/.github/workflows/documentation_generation.yml b/.github/workflows/documentation_generation.yml index 1d00c2e..31b1efb 100644 --- a/.github/workflows/documentation_generation.yml +++ b/.github/workflows/documentation_generation.yml @@ -18,12 +18,12 @@ jobs: steps: - name: Clone - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: HTML documentation run: doxygen .doxygen/Doxyfile - - uses: actions/upload-artifact@v3 + - uses: actions/upload-artifact@v4 with: name: documentation path: doc/html diff --git a/.github/workflows/misspellings_checks.yml b/.github/workflows/misspellings_checks.yml index 3365faa..4f802ab 100644 --- a/.github/workflows/misspellings_checks.yml +++ b/.github/workflows/misspellings_checks.yml @@ -1,6 +1,6 @@ -name: Misspellings checks +name: Checks on the Python client -# This workflow performs some misspelling checks on the repository +# This workflow performs some checks on the Python client used by the ragger tests # It is there to help us maintain a level of quality in our codebase and does not have to be kept on forked # applications. @@ -14,17 +14,28 @@ on: pull_request: jobs: - misspell: - name: Check misspellings + lint: + name: Client linting runs-on: ubuntu-latest steps: - - name: Clone - uses: actions/checkout@v3 - - - name: Check misspellings - uses: codespell-project/actions-codespell@v1 - with: - builtin: clear,rare - check_filenames: true - skip: ./.git,./unit-tests/mock_includes + - name: Clone + uses: actions/checkout@v4 + - name: Installing PIP dependencies + run: | + pip install pylint + pip install -r tests/requirements.txt + - name: Lint Python code + run: pylint --rc tests/setup.cfg tests/application_client/ + mypy: + name: Type checking + runs-on: ubuntu-latest + steps: + - name: Clone + uses: actions/checkout@v4 + - name: Installing PIP dependencies + run: | + pip install mypy + pip install -r tests/requirements.txt + - name: Mypy type checking + run: mypy tests/application_client/ diff --git a/.github/workflows/swap_ci_workflow.yml b/.github/workflows/swap_ci_workflow.yml new file mode 100644 index 0000000..4eec8e5 --- /dev/null +++ b/.github/workflows/swap_ci_workflow.yml @@ -0,0 +1,18 @@ +name: Swap functional tests + +on: + workflow_dispatch: + push: + branches: + - master + - main + - develop + pull_request: + +jobs: + job_functional_tests: + uses: LedgerHQ/app-exchange/.github/workflows/reusable_swap_functional_tests.yml@develop + with: + branch_for_kas: ${{ github.ref }} + repo_for_kas: ${{ github.repository }} + test_filter: '"KAS or Kaspa or kaspa"' diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index 09fd099..fc90b30 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -18,11 +18,18 @@ jobs: steps: - name: Clone - uses: actions/checkout@v3 + uses: actions/checkout@v4 + + - name: Clone SDK + uses: actions/checkout@v4 + with: + repository: ledgerHQ/ledger-secure-sdk + path: sdk - name: Build unit tests run: | cd unit-tests/ + export BOLOS_SDK=../sdk cmake -Bbuild -H. && make -C build && make -C build test - name: Generate code coverage @@ -31,18 +38,22 @@ jobs: lcov --directory . -b "$(realpath build/)" --capture --initial -o coverage.base && \ lcov --rc lcov_branch_coverage=1 --directory . -b "$(realpath build/)" --capture -o coverage.capture && \ lcov --directory . -b "$(realpath build/)" --add-tracefile coverage.base --add-tracefile coverage.capture -o coverage.info && \ - lcov --directory . -b "$(realpath build/)" --remove coverage.info '*/unit-tests/*' --remove coverage.info 'lib_standard_app' --remove coverage.info '*/src/import/*' -o coverage.info && \ + lcov --directory . -b "$(realpath build/)" --remove coverage.info '*/unit-tests/*' -o coverage.info && \ genhtml coverage.info -o coverage - - uses: actions/upload-artifact@v3 + - uses: actions/upload-artifact@v4 with: name: code-coverage path: unit-tests/coverage + - name: Install codecov dependencies + run: apt install --no-install-recommends -y curl gpg + - name: Upload to codecov.io - uses: codecov/codecov-action@v3 + uses: codecov/codecov-action@v5 + env: + CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} with: - token: ${{ secrets.CODECOV_TOKEN }} files: ./unit-tests/coverage.info flags: unittests name: codecov-app-boilerplate diff --git a/Makefile b/Makefile index 893737a..bff284d 100644 --- a/Makefile +++ b/Makefile @@ -29,8 +29,8 @@ APPNAME = "Kaspa" # Application version APPVERSION_M = 1 -APPVERSION_N = 0 -APPVERSION_P = 3 +APPVERSION_N = 1 +APPVERSION_P = 0 APPVERSION = "$(APPVERSION_M).$(APPVERSION_N).$(APPVERSION_P)" ifeq ($(TARGET_NAME),TARGET_NANOS) @@ -95,6 +95,7 @@ VARIANT_VALUES = KAS # Application communication interfaces # ######################################## ENABLE_BLUETOOTH = 1 +ENABLE_SWAP = 1 #ENABLE_NFC = 1 ######################################## diff --git a/ledger_app.toml b/ledger_app.toml index 216d37a..b52d502 100644 --- a/ledger_app.toml +++ b/ledger_app.toml @@ -1,7 +1,7 @@ [app] build_directory = "./" sdk = "C" -devices = ["flex", "nanos", "nanox", "nanos+", "stax"] +devices = ["flex", "nanox", "nanos+", "stax"] [tests] unit_directory = "./unit-tests/" diff --git a/src/address.c b/src/address.c index f61a5cb..94da79b 100644 --- a/src/address.c +++ b/src/address.c @@ -101,7 +101,13 @@ bool address_from_pubkey(const uint8_t public_key[static 64], memmove(address, hrp, sizeof(hrp)); address[5] = ':'; - cashaddr_encode(compressed_public_key, compressed_pub_size, address + 6, address_len, version); + if (cashaddr_encode(compressed_public_key, + compressed_pub_size, + address + 6, + address_len, + version) == 0) { + return false; + } memmove(out, address, address_len); diff --git a/src/app_main.c b/src/app_main.c index 5a2f5b1..dc6eeb1 100644 --- a/src/app_main.c +++ b/src/app_main.c @@ -21,19 +21,20 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. *****************************************************************************/ + #include // uint*_t #include // memset, explicit_bzero #include "os.h" #include "ux.h" +#include "swap.h" #include "types.h" #include "globals.h" #include "io.h" #include "sw.h" -#include "ui/menu.h" -#include "parser.h" -#include "apdu/dispatcher.h" +#include "menu.h" +#include "dispatcher.h" global_ctx_t G_context; @@ -48,7 +49,14 @@ void app_main() { io_init(); - ui_menu_main(); +#ifdef HAVE_SWAP + // When called in swap context as a library, we don't want to show the menu + if (!G_called_from_swap) { +#endif + ui_menu_main(); +#ifdef HAVE_SWAP + } +#endif // Reset context explicit_bzero(&G_context, sizeof(G_context)); @@ -82,4 +90,4 @@ void app_main() { return; } } -} \ No newline at end of file +} diff --git a/src/crypto.c b/src/crypto.c index a429f32..9e574cb 100644 --- a/src/crypto.c +++ b/src/crypto.c @@ -67,7 +67,7 @@ int crypto_sign_transaction(void) { G_context.bip32_path[0] = 0x8000002C; G_context.bip32_path[1] = 0x8001b207; G_context.bip32_path[2] = G_context.tx_info.transaction.account; - G_context.bip32_path[3] = (uint32_t)(txin->address_type); + G_context.bip32_path[3] = (uint32_t) (txin->address_type); G_context.bip32_path[4] = txin->address_index; G_context.bip32_path_len = 5; diff --git a/src/handler/sign_msg.c b/src/handler/sign_msg.c index 9ada060..2370363 100644 --- a/src/handler/sign_msg.c +++ b/src/handler/sign_msg.c @@ -97,7 +97,7 @@ int handler_sign_msg(buffer_t *cdata) { G_context.bip32_path[0] = 0x8000002C; G_context.bip32_path[1] = 0x8001b207; G_context.bip32_path[2] = G_context.msg_info.account; - G_context.bip32_path[3] = (uint32_t)(G_context.msg_info.address_type); + G_context.bip32_path[3] = (uint32_t) (G_context.msg_info.address_type); G_context.bip32_path[4] = G_context.msg_info.address_index; G_context.bip32_path_len = 5; diff --git a/src/handler/sign_tx.c b/src/handler/sign_tx.c index 36139ac..29dbea6 100644 --- a/src/handler/sign_tx.c +++ b/src/handler/sign_tx.c @@ -28,19 +28,61 @@ #include "os.h" #include "cx.h" +#include "buffer.h" +#include "swap.h" // G_swap_response_ready, G_called_from_swap #include "sign_tx.h" +#include "sw.h" +#include "globals.h" +#include "display.h" #include "constants.h" -#include "../apdu/dispatcher.h" -#include "../sw.h" -#include "../globals.h" -#include "../crypto.h" -#include "../ui/display.h" -#include "buffer.h" +#include "dispatcher.h" + +#include "crypto.h" #include "../transaction/types.h" #include "../transaction/deserialize.h" #include "../transaction/tx_validate.h" +#include "../transaction/utils.h" #include "../helper/send_response.h" +#include "handle_swap.h" +#include "validate.h" + +#ifdef HAVE_SWAP +static int check_and_sign_swap_tx(transaction_t *tx) { + if (G_swap_response_ready) { + // Safety against trying to make the app sign multiple TX + // This code should never be triggered as the app is supposed to exit after + // sending the signed transaction + PRINTF("Safety against double signing triggered\n"); + os_sched_exit(-1); + } else { + // We will quit the app after this transaction, whether it succeeds or fails + PRINTF("Swap response is ready, the app will quit after the next send\n"); + // This boolean will make the io_send_sw family instant reply + + // return to exchange + G_swap_response_ready = true; + } + uint64_t value = tx->tx_outputs[0].value; + uint64_t fees = calc_fees(tx->tx_inputs, tx->tx_input_len, tx->tx_outputs, tx->tx_output_len); + uint8_t to[ECDSA_ADDRESS_LEN + 1] = {0}; + if (!script_public_key_to_address(to, + sizeof(to), + tx->tx_outputs[0].script_public_key, + sizeof(tx->tx_outputs[0].script_public_key))) { + // No need to exit here early since `to` would be all zeros if this failed + // and will never match any valid kaspa address + PRINTF("Failed to convert script public key to address\n"); + } + + if (swap_check_validity(value, fees, to)) { + PRINTF("Swap response validated\n"); + validate_transaction(true); + } + // Unreachable because swap_check_validity() returns an error to exchange app OR + // validate_transaction() returns a success to exchange + return 0; +} +#endif // HAVE_SWAP static int sign_input_and_send() { int error = crypto_sign_transaction(); @@ -139,6 +181,14 @@ int handler_sign_tx(buffer_t *cdata, uint8_t type, bool more) { // last APDU for this transaction, let's parse, display and request a sign confirmation G_context.state = STATE_PARSED; +#ifdef HAVE_SWAP + // If we are in swap context, do not redisplay the message data + // Instead, ensure they are identical with what was previously displayed + if (G_called_from_swap) { + return check_and_sign_swap_tx(&G_context.tx_info.transaction); + } +#endif // HAVE_SWAP + return ui_display_transaction(); } } else if (type == 3) { diff --git a/src/sw.h b/src/sw.h index 7adffdf..5c2a9fd 100644 --- a/src/sw.h +++ b/src/sw.h @@ -94,3 +94,12 @@ #define SW_MESSAGE_ADDRESS_INDEX_FAIL 0xB014 #define SW_MESSAGE_LEN_PARSING_FAIL 0xB015 #define SW_MESSAGE_UNEXPECTED 0xB016 + +/** + * Status word for swap failure + */ +#define SW_SWAP_FAIL 0xC000 +/** + * Application specific swap error code + */ +#define SWAP_ERROR_CODE 0x00 diff --git a/src/swap/handle_check_address.c b/src/swap/handle_check_address.c new file mode 100644 index 0000000..4282288 --- /dev/null +++ b/src/swap/handle_check_address.c @@ -0,0 +1,149 @@ +#ifdef HAVE_SWAP +#include "swap.h" +#include "buffer.h" +#include "bip32.h" +#include "crypto_helpers.h" +#include "cx.h" +#include "os.h" + +#include "types.h" +#include "format.h" +#include "address.h" +#include "../transaction/types.h" // SCHNORR_ADDRESS_LEN, ECDSA_ADDRESS_LEN + +#include + +/* Check that the address used to receive funds is owned by the device + * check_address_parameters_t is defined in C SDK as: + * struct { + * // IN + * uint8_t *coin_configuration; + * uint8_t coin_configuration_length; + * // serialized path, segwit, version prefix, hash used, dictionary etc. + * // fields and serialization format depends on specific coin app + * uint8_t *address_parameters; + * uint8_t address_parameters_length; + * char *address_to_check; + * char *extra_id_to_check; + * // OUT + * int result; + * } check_address_parameters_t; + */ + +static int read_and_validate_bip32_path(const uint8_t *address_parameters, + uint8_t address_parameters_length, + uint32_t *bip32_path, + uint8_t *bip32_path_len) { + if (address_parameters == NULL) { + PRINTF("derivation path expected\n"); + return 0; + } + + buffer_t buf = {.ptr = address_parameters, .size = address_parameters_length, .offset = 0}; + + buffer_read_u8(&buf, bip32_path_len); + buffer_read_bip32_path(&buf, bip32_path, (size_t) *bip32_path_len); + + // Display the BIP32 path immediately after reading + PRINTF("BIP32 Path: "); + for (int i = 0; i < *bip32_path_len; i++) { + PRINTF("%d", bip32_path[i] & 0x7FFFFFFF); + if (bip32_path[i] & 0x80000000) { + PRINTF("'"); + } + if (i < *bip32_path_len - 1) { + PRINTF("/"); + } + } + PRINTF("\n"); + + // Now perform validation checks + if (*bip32_path_len < 2 || *bip32_path_len > KASPA_MAX_BIP32_PATH_LEN) { + PRINTF("BIP32 path length must be in range [2,5]. Invalid bip32 path length %d\n", + *bip32_path_len); + return 0; + } + + // Validate if bip32_path is bip32_path[0] = 0x8000002C and bip32_path[1] = 0x8001b207 + if (bip32_path[0] != 0x8000002C || bip32_path[1] != 0x8001b207) { + PRINTF("Invalid bip32 path\n"); + return 0; + } + + return 1; +} + +void swap_handle_check_address(check_address_parameters_t *params) { + PRINTF("Inside swap_handle_check_address\n"); + params->result = 0; + + if (params->address_parameters == NULL) { + PRINTF("derivation path expected\n"); + return; + } + PRINTF("address_parameters %.*H\n", + params->address_parameters_length, + params->address_parameters); + + if (params->address_to_check == NULL) { + PRINTF("Address to check expected\n"); + return; + } + + // The address_to_check here is the refund address. We guarantee that it is something we + // can derived from the bip32 path. + PRINTF("Address to check %s\n", params->address_to_check); + if (strlen(params->address_to_check) != SCHNORR_ADDRESS_LEN) { + PRINTF("Address to check expected length %d or %d, not %d\n", + SCHNORR_ADDRESS_LEN, + ECDSA_ADDRESS_LEN, + strlen(params->address_to_check)); + return; + } + + uint8_t bip32_path_len; + uint32_t bip32_path[MAX_BIP32_PATH]; + pubkey_ctx_t pk_info = {0}; + + if (!read_and_validate_bip32_path(params->address_parameters, + params->address_parameters_length, + bip32_path, + &bip32_path_len)) { + return; + } + + uint8_t raw_pubkey[65] = {0}; + cx_err_t ret = bip32_derive_get_pubkey_256(CX_CURVE_256K1, + bip32_path, + bip32_path_len, + raw_pubkey + 1, + pk_info.chain_code, + CX_SHA512); + memmove(pk_info.raw_public_key, raw_pubkey + 1, 64); + + if (ret != CX_OK) { + PRINTF("Failed to derive public key\n"); + return; + } + + uint8_t address[SCHNORR_ADDRESS_LEN] = {0}; + if (!address_from_pubkey(pk_info.raw_public_key, SCHNORR, address, sizeof(address))) { + PRINTF("Failed to convert public key to address\n"); + return; + } + + char derived_address[SCHNORR_ADDRESS_LEN + 1]; + memset(derived_address, 0, sizeof(derived_address)); + snprintf(derived_address, sizeof(derived_address), "%.*s", sizeof(address), address); + PRINTF("Derived address %s\n", address); + + PRINTF("Checked address %s\n", params->address_to_check); + + if (strncmp(derived_address, params->address_to_check, sizeof(derived_address)) != 0) { + PRINTF("Addresses do not match\n"); + } else { + PRINTF("Addresses match\n"); + params->result = 1; + } +} +#endif // HAVE_SWAP diff --git a/src/swap/handle_get_printable_amount.c b/src/swap/handle_get_printable_amount.c new file mode 100644 index 0000000..d0f10d4 --- /dev/null +++ b/src/swap/handle_get_printable_amount.c @@ -0,0 +1,48 @@ +#ifdef HAVE_SWAP +#include "swap.h" +#include "buffer.h" +#include "constants.h" +#include "format.h" +#include "os.h" + +#include +#include + +/* Format printable amount including the ticker from specified parameters. + * + * Must set empty printable_amount on error, printable amount otherwise + * get_printable_amount_parameters_t is defined in C SDK as: + * struct { + * // IN + * uint8_t *coin_configuration; + * uint8_t coin_configuration_length; + * uint8_t *amount; + * uint8_t amount_length; + * bool is_fee; + * // OUT + * char printable_amount[MAX_PRINTABLE_AMOUNT_SIZE]; + * } get_printable_amount_parameters_t; + */ +void swap_handle_get_printable_amount(get_printable_amount_parameters_t* params) { + PRINTF("Inside swap_handle_get_printable_amount\n"); + + PRINTF("Amount: %.*H\n", params->amount_length, params->amount); + + char amount[30] = {0}; + memset(params->printable_amount, 0, sizeof(params->printable_amount)); + + /// Convert params->amount into uint64_t + uint64_t value = 0; + if (swap_str_to_u64(params->amount, params->amount_length, &value)) { + format_fpu64_trimmed(amount, sizeof(amount), value, EXPONENT_SMALLEST_UNIT); + PRINTF("Formatted amount: %s\n", amount); + snprintf(params->printable_amount, + sizeof(params->printable_amount), + "KAS %.*s", + sizeof(amount), + amount); + } else { + PRINTF("Failed to convert amount to uint64_t\n"); + } +} +#endif // HAVE_SWAP diff --git a/src/swap/handle_swap.h b/src/swap/handle_swap.h new file mode 100644 index 0000000..f640416 --- /dev/null +++ b/src/swap/handle_swap.h @@ -0,0 +1,2 @@ +#pragma once +bool swap_check_validity(uint64_t amount, uint64_t fee, const uint8_t *destination); diff --git a/src/swap/handle_swap_sign_transaction.c b/src/swap/handle_swap_sign_transaction.c new file mode 100644 index 0000000..95f5fd3 --- /dev/null +++ b/src/swap/handle_swap_sign_transaction.c @@ -0,0 +1,176 @@ +#ifdef HAVE_SWAP +#include "swap.h" +#include "swap_error_code_helpers.h" +#include "os.h" +#include "format.h" + +#include "sw.h" +#include "globals.h" +#include "types.h" +#include "handle_swap.h" + +#include + +typedef struct swap_validated_s { + bool initialized; + uint64_t amount; + uint64_t fee; + char recipient[ECDSA_ADDRESS_LEN + 1]; // A standard address can be max 69 in len +} swap_validated_t; + +/* Global variable used to store swap validation status */ +static swap_validated_t G_swap_validated; + +static const char *charset = "qpzry9x8gf2tvdw0s3jn54khce6mua7l"; + +static bool is_valid_charset_char(char c) { + for (size_t i = 0; i < strlen(charset); i++) { + if (c == charset[i]) { + return true; + } + } + return false; +} + +static bool is_valid_address_charset(const char *addr, bool has_prefix) { + const char *addr_to_check = has_prefix ? addr + 6 : addr; + size_t len = strlen(addr_to_check); + + for (size_t i = 0; i < len; i++) { + if (!is_valid_charset_char(addr_to_check[i])) { + return false; + } + } + return true; +} + +bool swap_copy_transaction_parameters(create_transaction_parameters_t *params) { + PRINTF("Inside swap_copy_transaction_parameters %s\n", params->destination_address); + + if (params->destination_address == NULL) { + PRINTF("Destination address expected\n"); + return false; + } + + // Destination address is "normalized" by normalize_address from app-exchange, + // meaning if an address has a ':', it only takes the characters after the ':'. + // So we check also if the address has "kaspa:" prefix or not. + // Only minimal checks are necessary here since later when we actually convert the SPK + // if the saved address here is not valid, it will never match. + int len = strlen(params->destination_address); + bool has_prefix = (strncmp(params->destination_address, "kaspa:", 6) == 0); + bool has_valid_len_when_has_prefix = + has_prefix && (len == ECDSA_ADDRESS_LEN || len == SCHNORR_ADDRESS_LEN); + bool has_valid_len_when_prefix_removed = + !has_prefix && (len + 6 == ECDSA_ADDRESS_LEN || len + 6 == SCHNORR_ADDRESS_LEN); + if (!has_valid_len_when_has_prefix && !has_valid_len_when_prefix_removed) { + PRINTF("Destination address wrong length of %d\n", strlen(params->destination_address)); + return false; + } + + // Check charset validity only if length checks passed + if (!is_valid_address_charset(params->destination_address, has_prefix)) { + PRINTF("Destination address contains invalid characters\n"); + return false; + } + + if (params->amount == NULL) { + PRINTF("Amount expected\n"); + return false; + } + + // first copy parameters to stack, and then to global data. + // We need this "trick" as the input data position can overlap with app globals + // and also because we want to memset the whole bss segment as it is not done + // when an app is called as a lib. + // This is necessary as many part of the code expect bss variables to + // initialized at 0. + swap_validated_t swap_validated; + memset(&swap_validated, 0, sizeof(swap_validated)); + + // Save recipient as is. Add "kaspa:" prefix if missing + // This logic depends on the normalize_address function in app-exchange + // which removes the "kaspa:" prefix part due to the ':'. + // See this line: + // https://github.com/LedgerHQ/app-exchange/blob/fc2f1d6578db5fe778c6343a359b38d7fb730cd2/src/process_transaction.c#L288 + // The implementation here is robust against changes to this normalize_address + if (!has_prefix) { + strncpy(swap_validated.recipient, "kaspa:", sizeof(swap_validated.recipient)); + strncpy(swap_validated.recipient + 6, + params->destination_address, + sizeof(swap_validated.recipient) - 6); + } else { + strncpy(swap_validated.recipient, + params->destination_address, + sizeof(swap_validated.recipient)); + } + + // Save amount + if (!swap_str_to_u64(params->amount, params->amount_length, &swap_validated.amount)) { + PRINTF("Failed to convert amount to uint64_t\n"); + return false; + } + + // Save fee + if (!swap_str_to_u64(params->fee_amount, params->fee_amount_length, &swap_validated.fee)) { + PRINTF("Failed to convert fee to uint64_t\n"); + return false; + } + + swap_validated.initialized = true; + + // Full reset the global variables + os_explicit_zero_BSS_segment(); + + // Commit from stack to global data, params becomes tainted but we won't access it anymore + memcpy(&G_swap_validated, &swap_validated, sizeof(swap_validated)); + + PRINTF("Out of swap_copy_transaction_parameters\n"); + + return true; +} + +/* Check if the Tx to sign have the same parameters as the ones previously validated */ +bool swap_check_validity(uint64_t amount, uint64_t fee, const uint8_t *destination) { + PRINTF("Inside swap_check_validity\n"); + + if (!G_swap_validated.initialized) { + PRINTF("Swap structure is not initialized\n"); + send_swap_error_simple(SW_SWAP_FAIL, SWAP_EC_ERROR_GENERIC, SWAP_ERROR_CODE); + // unreachable + os_sched_exit(0); + } + + if (G_swap_validated.amount != amount) { + PRINTF("Amount does not match, promised %lld, received %lld\n", + G_swap_validated.amount, + amount); + send_swap_error_simple(SW_SWAP_FAIL, SWAP_EC_ERROR_WRONG_AMOUNT, SWAP_ERROR_CODE); + // unreachable + os_sched_exit(0); + } else { + PRINTF("Amounts match \n"); + } + + if (G_swap_validated.fee != fee) { + PRINTF("Fee does not match, promised %lld, received %lld\n", G_swap_validated.fee, fee); + send_swap_error_simple(SW_SWAP_FAIL, SWAP_EC_ERROR_WRONG_FEES, SWAP_ERROR_CODE); + // unreachable + os_sched_exit(0); + } else { + PRINTF("Fees match \n"); + } + + if (strcmp(G_swap_validated.recipient, (const char *) destination) != 0) { + PRINTF("Destination does not match\n"); + PRINTF("Validated: %s\n", G_swap_validated.recipient); + PRINTF("Received: %s \n", destination); + send_swap_error_simple(SW_SWAP_FAIL, SWAP_EC_ERROR_WRONG_DESTINATION, SWAP_ERROR_CODE); + // unreachable + os_sched_exit(0); + } else { + PRINTF("Destination is valid\n"); + } + return true; +} +#endif // HAVE_SWAP diff --git a/src/transaction/deserialize.c b/src/transaction/deserialize.c index 32747ff..3c49168 100644 --- a/src/transaction/deserialize.c +++ b/src/transaction/deserialize.c @@ -195,7 +195,7 @@ parser_status_e transaction_deserialize(buffer_t *buf, transaction_t *tx, uint32 bip32_path[0] = 0x8000002C; bip32_path[1] = 0x8001b207; bip32_path[2] = tx->account; - bip32_path[3] = (uint32_t)(change_address_type); + bip32_path[3] = (uint32_t) (change_address_type); bip32_path[4] = change_address_index; return buf->size - buf->offset == 0 ? PARSING_OK : HEADER_PARSING_ERROR; diff --git a/src/transaction/utils.h b/src/transaction/utils.h index 436129b..76abe12 100644 --- a/src/transaction/utils.h +++ b/src/transaction/utils.h @@ -51,7 +51,7 @@ bool transaction_utils_check_encoding(const uint8_t* memo, uint64_t memo_len); * Pointer to the buffer to read script_public_key from * @param[in] script_len length of script output buffer */ -void script_public_key_to_address(uint8_t* out_address, +bool script_public_key_to_address(uint8_t* out_address, size_t out_len, uint8_t* in_script_public_key, size_t script_len); diff --git a/src/ui/bagl_display.c b/src/ui/bagl_display.c index 5512600..382bc91 100644 --- a/src/ui/bagl_display.c +++ b/src/ui/bagl_display.c @@ -215,11 +215,13 @@ int ui_display_transaction() { uint8_t address[ECDSA_ADDRESS_LEN] = {0}; - script_public_key_to_address( - address, - sizeof(address), - G_context.tx_info.transaction.tx_outputs[0].script_public_key, - sizeof(G_context.tx_info.transaction.tx_outputs[0].script_public_key)); + if (!script_public_key_to_address( + address, + sizeof(address), + G_context.tx_info.transaction.tx_outputs[0].script_public_key, + sizeof(G_context.tx_info.transaction.tx_outputs[0].script_public_key))) { + return io_send_sw(SW_DISPLAY_ADDRESS_FAIL); + } snprintf(g_address, sizeof(g_address), "%.*s", ECDSA_ADDRESS_LEN, address); g_validate_callback = &ui_action_validate_transaction; diff --git a/src/ui/nbgl_display_transaction.c b/src/ui/nbgl_display_transaction.c old mode 100755 new mode 100644 index 23c6c53..e2542b1 --- a/src/ui/nbgl_display_transaction.c +++ b/src/ui/nbgl_display_transaction.c @@ -102,11 +102,13 @@ int ui_display_transaction() { uint8_t address[ECDSA_ADDRESS_LEN] = {0}; - script_public_key_to_address( - address, - sizeof(address), - G_context.tx_info.transaction.tx_outputs[0].script_public_key, - sizeof(G_context.tx_info.transaction.tx_outputs[0].script_public_key)); + if (!script_public_key_to_address( + address, + sizeof(address), + G_context.tx_info.transaction.tx_outputs[0].script_public_key, + sizeof(G_context.tx_info.transaction.tx_outputs[0].script_public_key))) { + return io_send_sw(SW_DISPLAY_ADDRESS_FAIL); + } snprintf(g_address, sizeof(g_address), "%.*s", ECDSA_ADDRESS_LEN, address); // Setup data to display diff --git a/tests/application_client/kaspa_command_sender.py b/tests/application_client/kaspa_command_sender.py index c6abd06..b50528e 100644 --- a/tests/application_client/kaspa_command_sender.py +++ b/tests/application_client/kaspa_command_sender.py @@ -128,7 +128,6 @@ def sign_tx(self, transaction: Transaction) -> Generator[None, None, None]: p2=P2.P2_MORE, data=txoutput.serialize()) - txinput = None # used again after the loop for the last value for i, txinput in enumerate(transaction.inputs): if i < len(transaction.inputs) - 1: self.backend.exchange(cla=CLA, @@ -136,15 +135,15 @@ def sign_tx(self, transaction: Transaction) -> Generator[None, None, None]: p1=P1.P1_INPUTS, p2=P2.P2_MORE, data=txinput.serialize()) - - # Last input, we'll end here - with self.backend.exchange_async(cla=CLA, - ins=InsType.SIGN_TX, - p1=P1.P1_INPUTS, - p2=P2.P2_LAST, - data=txinput.serialize()) as response: - - yield response + else: + # Last input, we'll end here + with self.backend.exchange_async(cla=CLA, + ins=InsType.SIGN_TX, + p1=P1.P1_INPUTS, + p2=P2.P2_LAST, + data=txinput.serialize()) as response: + + yield response @contextmanager def sign_message(self, message_data: PersonalMessage) -> Generator[None, None, None]: diff --git a/tests/requirements.txt b/tests/requirements.txt index 9ed04a8..a8c3aab 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -1,8 +1,3 @@ pytest -ragger[speculos]>=1.6.0 -ragger[ledgerwallet]>=1.6.0 -Jinja2==3.1.2 -Flask==2.1.2 -pysha3>=1.0.0,<2.0.0 -secp256k1>=0.14.0 -Werkzeug==2.2.2 \ No newline at end of file +ragger[speculos,ledgerwallet]>=1.6.0 +secp256k1>=0.14.0 \ No newline at end of file diff --git a/tests/snapshots/flex/test_app_mainmenu/00001.png b/tests/snapshots/flex/test_app_mainmenu/00001.png index 9bc0d7e..bc883de 100644 Binary files a/tests/snapshots/flex/test_app_mainmenu/00001.png and b/tests/snapshots/flex/test_app_mainmenu/00001.png differ diff --git a/tests/snapshots/flex/test_get_public_key_confirm_accepted/00000.png b/tests/snapshots/flex/test_get_public_key_confirm_accepted/00000.png index 59fef6b..c14ff29 100644 Binary files a/tests/snapshots/flex/test_get_public_key_confirm_accepted/00000.png and b/tests/snapshots/flex/test_get_public_key_confirm_accepted/00000.png differ diff --git a/tests/snapshots/flex/test_get_public_key_confirm_accepted/00001.png b/tests/snapshots/flex/test_get_public_key_confirm_accepted/00001.png index c5d7b01..9b06332 100644 Binary files a/tests/snapshots/flex/test_get_public_key_confirm_accepted/00001.png and b/tests/snapshots/flex/test_get_public_key_confirm_accepted/00001.png differ diff --git a/tests/snapshots/flex/test_get_public_key_confirm_accepted/00002.png b/tests/snapshots/flex/test_get_public_key_confirm_accepted/00002.png index cf4307a..4321e60 100644 Binary files a/tests/snapshots/flex/test_get_public_key_confirm_accepted/00002.png and b/tests/snapshots/flex/test_get_public_key_confirm_accepted/00002.png differ diff --git a/tests/snapshots/flex/test_get_public_key_confirm_accepted/00003.png b/tests/snapshots/flex/test_get_public_key_confirm_accepted/00003.png index 4321e60..26a62a4 100644 Binary files a/tests/snapshots/flex/test_get_public_key_confirm_accepted/00003.png and b/tests/snapshots/flex/test_get_public_key_confirm_accepted/00003.png differ diff --git a/tests/snapshots/flex/test_get_public_key_confirm_refused/00000.png b/tests/snapshots/flex/test_get_public_key_confirm_refused/00000.png index 59fef6b..c14ff29 100644 Binary files a/tests/snapshots/flex/test_get_public_key_confirm_refused/00000.png and b/tests/snapshots/flex/test_get_public_key_confirm_refused/00000.png differ diff --git a/tests/snapshots/flex/test_get_public_key_confirm_refused/00001.png b/tests/snapshots/flex/test_get_public_key_confirm_refused/00001.png index c5d7b01..9b06332 100644 Binary files a/tests/snapshots/flex/test_get_public_key_confirm_refused/00001.png and b/tests/snapshots/flex/test_get_public_key_confirm_refused/00001.png differ diff --git a/tests/snapshots/flex/test_get_public_key_confirm_refused/00002.png b/tests/snapshots/flex/test_get_public_key_confirm_refused/00002.png index cf4307a..45c08d4 100644 Binary files a/tests/snapshots/flex/test_get_public_key_confirm_refused/00002.png and b/tests/snapshots/flex/test_get_public_key_confirm_refused/00002.png differ diff --git a/tests/snapshots/flex/test_get_public_key_confirm_refused/00003.png b/tests/snapshots/flex/test_get_public_key_confirm_refused/00003.png index 45c08d4..26a62a4 100644 Binary files a/tests/snapshots/flex/test_get_public_key_confirm_refused/00003.png and b/tests/snapshots/flex/test_get_public_key_confirm_refused/00003.png differ diff --git a/tests/snapshots/nanosp/test_app_mainmenu/00001.png b/tests/snapshots/nanosp/test_app_mainmenu/00001.png index ea02a72..7e64591 100644 Binary files a/tests/snapshots/nanosp/test_app_mainmenu/00001.png and b/tests/snapshots/nanosp/test_app_mainmenu/00001.png differ diff --git a/tests/snapshots/nanox/test_app_mainmenu/00001.png b/tests/snapshots/nanox/test_app_mainmenu/00001.png index ea02a72..7e64591 100644 Binary files a/tests/snapshots/nanox/test_app_mainmenu/00001.png and b/tests/snapshots/nanox/test_app_mainmenu/00001.png differ diff --git a/tests/snapshots/stax/test_app_mainmenu/00001.png b/tests/snapshots/stax/test_app_mainmenu/00001.png index 32de7d6..b9d10ed 100644 Binary files a/tests/snapshots/stax/test_app_mainmenu/00001.png and b/tests/snapshots/stax/test_app_mainmenu/00001.png differ diff --git a/tests/snapshots/stax/test_get_public_key_confirm_accepted/00000.png b/tests/snapshots/stax/test_get_public_key_confirm_accepted/00000.png index e25645c..57a43e7 100644 Binary files a/tests/snapshots/stax/test_get_public_key_confirm_accepted/00000.png and b/tests/snapshots/stax/test_get_public_key_confirm_accepted/00000.png differ diff --git a/tests/snapshots/stax/test_get_public_key_confirm_accepted/00001.png b/tests/snapshots/stax/test_get_public_key_confirm_accepted/00001.png index 6c9e58d..284a1ba 100644 Binary files a/tests/snapshots/stax/test_get_public_key_confirm_accepted/00001.png and b/tests/snapshots/stax/test_get_public_key_confirm_accepted/00001.png differ diff --git a/tests/snapshots/stax/test_get_public_key_confirm_accepted/00002.png b/tests/snapshots/stax/test_get_public_key_confirm_accepted/00002.png index f21a2a9..7a49478 100644 Binary files a/tests/snapshots/stax/test_get_public_key_confirm_accepted/00002.png and b/tests/snapshots/stax/test_get_public_key_confirm_accepted/00002.png differ diff --git a/tests/snapshots/stax/test_get_public_key_confirm_accepted/00003.png b/tests/snapshots/stax/test_get_public_key_confirm_accepted/00003.png index 7a49478..2519c9c 100644 Binary files a/tests/snapshots/stax/test_get_public_key_confirm_accepted/00003.png and b/tests/snapshots/stax/test_get_public_key_confirm_accepted/00003.png differ diff --git a/tests/snapshots/stax/test_get_public_key_confirm_refused/00000.png b/tests/snapshots/stax/test_get_public_key_confirm_refused/00000.png index e25645c..57a43e7 100644 Binary files a/tests/snapshots/stax/test_get_public_key_confirm_refused/00000.png and b/tests/snapshots/stax/test_get_public_key_confirm_refused/00000.png differ diff --git a/tests/snapshots/stax/test_get_public_key_confirm_refused/00001.png b/tests/snapshots/stax/test_get_public_key_confirm_refused/00001.png index 6c9e58d..284a1ba 100644 Binary files a/tests/snapshots/stax/test_get_public_key_confirm_refused/00001.png and b/tests/snapshots/stax/test_get_public_key_confirm_refused/00001.png differ diff --git a/tests/snapshots/stax/test_get_public_key_confirm_refused/00002.png b/tests/snapshots/stax/test_get_public_key_confirm_refused/00002.png index f21a2a9..94c91bb 100644 Binary files a/tests/snapshots/stax/test_get_public_key_confirm_refused/00002.png and b/tests/snapshots/stax/test_get_public_key_confirm_refused/00002.png differ diff --git a/tests/snapshots/stax/test_get_public_key_confirm_refused/00003.png b/tests/snapshots/stax/test_get_public_key_confirm_refused/00003.png index 94c91bb..2519c9c 100644 Binary files a/tests/snapshots/stax/test_get_public_key_confirm_refused/00003.png and b/tests/snapshots/stax/test_get_public_key_confirm_refused/00003.png differ diff --git a/tests/test_name_version.py b/tests/test_name_version.py index cafa7b5..81d291f 100644 --- a/tests/test_name_version.py +++ b/tests/test_name_version.py @@ -12,4 +12,4 @@ def test_get_app_and_version(backend, backend_name): app_name, version = unpack_get_app_and_version_response(response.data) assert app_name == "Kaspa" - assert version == "1.0.3" + assert version == "1.1.0" diff --git a/tests/test_version_cmd.py b/tests/test_version_cmd.py index ebcba95..84843ae 100644 --- a/tests/test_version_cmd.py +++ b/tests/test_version_cmd.py @@ -3,8 +3,8 @@ # Taken from the Makefile, to update every time the Makefile version is bumped MAJOR = 1 -MINOR = 0 -PATCH = 3 +MINOR = 1 +PATCH = 0 # In this test we check the behavior of the device when asked to provide the app version def test_version(backend): diff --git a/unit-tests/CMakeLists.txt b/unit-tests/CMakeLists.txt index 143df34..a620c26 100644 --- a/unit-tests/CMakeLists.txt +++ b/unit-tests/CMakeLists.txt @@ -33,16 +33,16 @@ if(${CMAKE_SOURCE_DIR} STREQUAL ${CMAKE_BINARY_DIR}) message(FATAL_ERROR "In-source builds not allowed. Please make a new directory (called a build directory) and run CMake from there. You may need to remove CMakeCache.txt. ") endif() -add_compile_definitions(TEST HAVE_HASH HAVE_BLAKE2 HAVE_ECC USB_SEGMENT_SIZE=64 MAX_INPUT_COUNT=15 MAX_MESSAGE_LEN=200 IO_SEPROXYHAL_BUFFER_SIZE_B=128) +add_compile_definitions(TEST HAVE_HASH HAVE_BLAKE2 HAVE_ECC USB_SEGMENT_SIZE=64 MAX_INPUT_COUNT=15 MAX_MESSAGE_LEN=200 IO_SEPROXYHAL_BUFFER_SIZE_B=128 OS_IO_SEPH_BUFFER_SIZE=272) -include_directories(../src) -# include_directories(mock_includes) include_directories(mock_includes) -# include_directories(/opt/nanos-secure-sdk/include/) include_directories(/opt/ledger-secure-sdk/include/) include_directories(/opt/ledger-secure-sdk/lib_cxng/include) include_directories(/opt/ledger-secure-sdk/lib_standard_app/) +include_directories(/opt/ledger-secure-sdk/io/include) +include_directories(/opt/ledger-secure-sdk/io_legacy/include/) +include_directories(../src) add_executable(test_address test_address.c) add_executable(test_format test_format.c) diff --git a/unit-tests/mock_includes/ux.h b/unit-tests/mock_includes/ux.h index 0e7a95c..932d6a6 100644 --- a/unit-tests/mock_includes/ux.h +++ b/unit-tests/mock_includes/ux.h @@ -1,3 +1,5 @@ #define CX_CURVE_256K1 0x21 -typedef struct ux_state_s ux_state_t; \ No newline at end of file +typedef struct ux_state_s ux_state_t; + +typedef struct bolos_ux_params_s bolos_ux_params_t; \ No newline at end of file