CertiK reviewed ChainPort Cross-Chain Bridge (ChainPort-Cardano Bridge) on 19 Dec 2022: 21 findings, 1 critical; 13 marked fixed, 8 open.
Report hosted on skynet.certik.com.
Trustblock does not review code.
Statuses are the security firm’s own, from its report.
| Severity | Open | Acknowledged | Fixed |
|---|---|---|---|
| Critical | 0 | 0 | 1 |
| High | 1 | 0 | 0 |
| Medium | 0 | 0 | 3 |
| Low | 7 | 0 | 9 |
The filter criteria for `session_get_pending_cardano_burn_transactions` is `CardanoTransaction.transaction_status == 0 and CardanoTransaction.transaction_type == "cardano_burn"`, which is using Python's built-in **and** keyword. In Python, when x and y are not both integers, **x and y** will return y if x is not None, and will return None if x is None. Since `CardanoTransaction.transaction_status == 0` is not `None`, the filter expression will evaluate to `CardanoTransaction.transaction_type == "cardano_burn"` and will ultimately fetch any burn transaction, regardless of its status. When using one filter call with multiple criteria, the proper function to use is the [SQLAlchemy-Defined `and_`](https://docs.sqlalchemy.org/en/14/core/sqlelement.html#sqlalchemy.sql.expression.and_), or separate each criteria with a comma.
`cardano_network_id` will always have either a value of `(None,)` (from its initialization) or the non-nullable integer from the `id` column of a row from the `Network` table (from the loop over `network_objs`). In both of these cases, `cardano_network_id is not None` will give `True` and therefore the check is unnecessary. ---- It is likely that the intent is for `cardano_network_id` to be initialized to `None` instead of `(None,)` in line 22. With this, if a suitable `cardano_network_id` is not found from looking in the `network_objs`, the `cardano_network_id is not None` condition will give `False`. However, if `cardano_network_id` is supposed to be `(None,)` then the `cardano_network_id is not None` condition will not be needed, as it will always be true.
The `conditional_fields` variable is initialized to hold a list of strings. This list is then checked to see if it contains `None`, but this will never be the case. Most likely, these strings should instead be the variables initialized above with the same names, except `cardano_token_address` which should instead be `cardano_user_address`.
The `network` argument in the constructor call for `BlockFrostChainContext` was removed in the latest version. The default value of `Network.TESTNET` for that parameter works when `self.env` is not `"prod"` since that's the default for the constructor, but when `self.env` is `"prod"`, it is not consistent with `self.network` (`Network.MAINNET`).
The method `burn_utxo()` returns `None` if the `utxo` does not contain multi asset token, or if its output is not corresponding to the burn address. \ With this, the execution of `cardano_process_utxo()` will fail causing `cardano_utxo_monitor()` to fail during a *for* loop.
The method `session_get_cardano_transaction_by_id()` can return `None`, but this may not be handled in the `cardano_burn_monitor()` method. \ This could lead to `cardano_to_evm_transaction()` being called with `None` as a parameter, causing the computation to fail. If this happens, it will repeat in all future calls to `cardano_burn_monitor()`, which will prevent this feature to work correctly.
In the method `cardano_transaction_monitor()`, the function `get_transaction_info()` is called and could return `None`. In the method `cardano_burn_monitor()`, there is a test to handle this potential case. \ However in `cardano_transaction_monitor()` this case is not handled. If the returned value is `None`, the execution will fail at the following *if* statement. Moreover, the transaction will stay in a pending state and block all future computations. Finally, because `inner_rw_session.commit()` is outside of the *for* loop, all the database uploads before this specific `transaction_info` are canceled. We understand that because the transaction is in a pending state, `get_transaction_info()` should not return `None`, however, the transaction hash uses as a parameter that is obtained from the `session_get_pending_transactions_by_network()` method, which is not in the audit scope and can therefore not be completely checked.
In the contract `cardano_monitor.py`, the method `cardano_to_release_transaction()` computes the `amount_in_wei` variable to determine the `withdrawal_amount_wei` parameter for the `signature` of the `redeem_obj`. \ However in the new version, `amount_in_wei` is not used directly. If there is no `redeem_obj`, it will be created and a new variable `redeem_amount` will be created. This new variable `redeem_amount` is then used to set the `withdrawal_amount_wei` of the `signature`: ```python=756 signature = main_bridge_obj.generate_redeem_signature( checksummed_benficiary_address=to_checksum_address(receiver_web3_identity_obj.web3_address), checksummed_token_address=to_checksum_address(target_token_obj.web3_address), withdrawal_amount_wei=redeem_amount, nonce=redeem_obj.target_tx_nonce, ) ``` This means that if `redeem_obj` is not `None`, `redeem_amount` will never be set and the `withdrawal_amount_wei` parameter will be an uninitialized variable.
In the method `generate_unsigned_transaction_with_metadata()`, the variable `asset_name` will include `[via ChainPort.io]` only if the related token does not have a specific name: ```python=456 if token_name == "HORD Token": asset_name = AssetName(token_name.encode("UTF-8")) else: asset_name = AssetName((token_name + " [via ChainPort.io]").encode("UTF-8")) ``` However, in the method `mint_token()`, the `cardano_token` will be set using the name including `[via ChainPort.io]` only if the related `token_obj` has a specific symbol: ```python=611 if token_obj.symbol == "COPI": cardano_token = AssetName((token_obj.name + " [via ChainPort.io]").encode("UTF-8")) else: cardano_token = AssetName((token_obj.name).encode("UTF-8")) ```
In the new `transaction.py`, file in the linked pieces of code, the variable `fname` is not consistent with the name of the method. This would make the exception messages harder to understand.
In the `cardano_monitor.py`, `cardano_utxo_monitor()` is the only lambda function handler that does not have the `@heartbeat` decorator.
In the method `cardano_to_evm_transaction()`, the `token_asset_name_hash` is extracted from the `utxo_output`, then `target_network_id` and `target_web3_address` are retrieved from `metadata` and finally `token_asset_name_hash` is checked.
The `metadata` variable is declared and initialized, but is not used. It is shadowed by the iterator variable with the same name in the **for** loop, which is used in the body of the loop.
The report names a repository or files. Open the report to see its scope.
1,044 projects reviewed · 1,375 reports · latest May 2025
See CertiK’s auditsSomething wrong on this page? A missing report, a wrong date, a finding that was fixed since. Tell us and we check it against its source.
Tell us