In the swapAndSendToAptos() function, the swapTarget address is utilized to execute the swap through OpenZeppelin’s Address library's functionCall() method, which returns a bytes value upon execution. However, the swapAndSendToAptos() function currently lacks verification of this return value from the swapTarget function call. This lack of verification could lead to unanticipated outcomes if the function call returns a value that is expected to be verified.

Handle the return value of the function call, especially if it is expected to contain critical data or if there is uncertainty about its content.

Remediation Plan

RISK ACCEPTED: The Taurus Labs team made a business decision to accept the risk of this finding and not alter the contracts, stating:

The target function of the invocation is one of the swap functions defined in the IParaswap interface. Checking the return value isn’t very practical since different swap functions in IParaswap have different return signatures. Also the value itself isn’t very useful for verification purpose and we can whether the swap has been executed correctly, which we verify by comparing the toToken balance and toAmount after the function call.

The SwapBridge contract inherits from OpenZeppelin's Ownable contract and declares the owner at deployment. This owner address is used to perform administrative actions on the contract, such as token withdrawals and pausing/unpausing contract functions.

This centralization of control over the contract may pose is a risk to the protocol, as this architecture may put the assets of users who have engaged with the protocol in a vulnerable position, exposing them to the potential threat of a rug pull. By centralizing control over the stored funds, the contract compromises on decentralization principles and elevates the risk where user assets could be abruptly withdrawn by the owner, leaving users with no recourse.

Several remedial strategies can be employed, including but not limited to: implementing role-based access control, creating low-privileged roles to execute daily tasks in the protocol to limit the impact of possible compromise, transitioning control to a Multi-Signature wallet setup, establishing community-driven governance for decision-making on fund management, integrating time locks and/or setting withdrawal limits to prevent abrupt fund depletion.

Remediation Plan

RISK ACCEPTED: The Taurus Labs team made a business decision to accept the risk of this finding and not alter the contracts, stating:

The contract is purposefully non-upgradeable in order to minimize centralization risk; the Owner has no power to affect change in the behavior of the contract, namely swapping and bridging.

The SwapBridge contract inherits from OpenZeppelin's Ownable contract and allows for single-step ownership transfer via the transferOwnership() function. Regarding this, it is crucial that the address to which ownership is transferred is verified to be active and willing to assume ownership responsibilities. Otherwise, the contract could be locked in a situation where it is no longer possible to make administrative changes to it.

Additionally, in the Ownable contract, the renounceOwnership() function allows to renounce to the owner permission. Renouncing ownership before transferring it would result in the contract having no owner, eliminating the ability to call privileged functions.

Consider using OpenZeppelin's Ownable2Step contract over the Ownable contract, or implementing similar two-step ownership transfer logic into the contract.

Additionally, it is recommended that the owner cannot call the renounceOwnership() function without first transferring ownership to another address. In addition, if a multi-signature wallet is used, the call to the renounceOwnership() function should be confirmed by most signers.

Remediation Plan

SOLVED: The Taurus Labs team has addressed the finding in commit b796692 by following the mentioned recommendation.

The SwapBridge contract relies heavily on the tokenBridge and swapTarget addresses, which are established at contract initialization, to operate effectively. The tokenBridge address plays a critical role in quoting fees for swaps and facilitating the transfer of tokens to the Aptos chain. Meanwhile, the swapTarget address is core for executing swaps within the swapAndSendToAptos() function.

This reliance on external addresses underscores the necessity of ensuring these addresses are secure and trusted, as they have substantial control over critical functions of the contract.

Ensure the integrity and security of these addresses to maintain the contract's functionality and safeguard against potential unexpected vulnerabilities.

Remediation Plan

ACKNOWLEDGED: The Taurus Labs team made a business decision to acknowledge this finding and not alter the contracts, stating:

To minimize the attack surface, we will ensure the users of SwapBridge contract to approve only the minimal allowance required for each instance a user uses the contract. Moreover, the contract shouldn’t hold any surplus token holdings with normal usage (rescue function is there only to rescue any balance that someone might mistakenly send to the contract).

From Solidity versions 0.8.20 through 0.8.24, the default target EVM version is set to Shanghai, which results in the generation of bytecode that includes PUSH0 opcodes. Starting with version 0.8.25, the default EVM version shifts to Cancun, introducing new opcodes for transient storage, TSTORE and TLOAD.

In this aspect, it is crucial to select the appropriate EVM version when it's intended to deploy contracts on networks other than the Ethereum mainnet, such as L2 chains, which may not support these opcodes. Failure to do so could lead to unsuccessful contract deployments or transaction execution issues.

Make sure to specify the target EVM version when using Solidity versions from 0.8.20 and above, especially if deploying to chains that may not support newly introduced opcodes. Additionally, it is crucial to stay informed about the opcode support of different chains to ensure smooth deployment and compatibility.

Remediation Plan

SOLVED: The Taurus Labs team has addressed the finding in commit b796692 by following the mentioned recommendation and setting a specific EVM version for compilation and deployment. Further testing is required to ensure cross-chain compatibility.

The current contract in scope has an unlocked compiler ^0.8.21. To ensure stability, it's important that contracts be deployed with the same compiler version and flags used during development and testing. Locking the pragma helps to ensure that contracts do not accidentally get deployed using another pragma. For example, an outdated pragma version might introduce bugs that affect the contract system negatively.

Lock the pragma version to the same version used during development and testing.

Remediation Plan

SOLVED: The Taurus Labs team has addressed the finding in commit b796692 by following the mentioned recommendation.

In Solidity development, replacing hard-coded revert message strings with the Error() syntax is an optimization strategy that can significantly reduce gas costs. Hard-coded strings, stored on the blockchain, increase the size and cost of deploying and executing contracts.

The Error() syntax allows for the definition of reusable, parameterized custom errors, leading to a more efficient use of storage and reduced gas consumption. This approach not only optimizes gas usage during deployment and interaction with the contract but also enhances code maintainability and readability by providing clearer, context-specific error information.

Consider replacing all revert strings with custom errors. For more reference, see here.

Remediation Plan

SOLVED: The Taurus Labs team has addressed the finding in commit b796692 by following the mentioned recommendation.

The state variables declared in the SwapBridge.sol contract are not explicitly declared with any visibility modifiers. By default, the Solidity compiler will assume any state variable declared in the contract as internal.

It is considered best practice to explicitly specify visibility to enhance clarity and prevent ambiguity. Clearly labeling the visibility of all variables and functions will help in maintaining clear and understandable code.

Explicitly define the visibility for each state variable within the contract.

Remediation Plan

SOLVED: The Taurus Labs team has addressed the finding in commit b796692 by following the mentioned recommendation

In Solidity development, emitting events is a recommended practice when state-changing functions are invoked. Currently, the SwapBridge contract lacks emission of events, besides the ones emitted during calls with third-party dependencies. This absence may hamper effective state tracking in off-chain monitoring systems.

Consider declaring and emitting events in the contract to enhance transparency and facilitate off-chain monitoring.

Remediation Plan

SOLVED: The Taurus Labs team has addressed the finding in commit b796692 by following the mentioned recommendation.