Layerswap Atomic Bridge Security Review Report

As a guarantee to ensure that LP release the user's funds, LPs can voluntarily stack a small portion of their funds in a Atomic Stake pool, if they fail to release users found this portion will be slashed. LPs with locked amounts in the atomic pool will be more favorable to users during the LP selection process.

In HashedTimeLockEther.sol, this slash can easily be forced by simply using a malicious contract that will always revert when receiving fund unless tx.origin == the attacker. This would allow him to call redeem and get his assets back in the destination chain after slashing the LP stack in the atomic pool, while preventing the LP to release the fund for the attacker.

function redeem(
    bytes32 Id,
    uint256 secret
) external _exists(Id) returns (bool) {
    HTLC storage htlc = contracts[Id];
 
    if (htlc.hashlock != sha256(abi.encodePacked(secret)))
        revert HashlockNotMatch(); // Ensure secret matches hashlock.
    if (htlc.claimed == 3 || htlc.claimed == 2) revert AlreadyClaimed();
 
    htlc.claimed = 3;
    htlc.secret = secret;
    (bool success, ) = htlc.srcReceiver.call{value: htlc.amount}("");
    if (!success) revert TransferFailed();
    emit TokenRedeemed(Id, msg.sender, secret, htlc.hashlock);
    return true;
}

Monitor whether the LP has tried to call redeem several times without success, since the LP have no interest in maliciously making this call fail, slash shouldn't be possible in this situation.

HashedTimeLockEther::commit and HashedTimeLockERC20::commit may overwrite an already existing HTLC (hashed time-locked contract) (line 188 in HashedTimeLockEther.sol and line 209 in HashedTimeLockERC20.sol).

Example:

1. An actor calls HashedTimeLockEther::lock for a specific Id which is then stored on line 313 into the contracts storage variable.
2. Later, HashedTimeLockEther::commit may be called where the same Id may be generated (line 185 HashedTimeLockEther.sol), which would then cause the protocol to overwrite (line 188 in HashedTimeLockEther.sol) the existing HTLC.
3. As a consequence funds may be lost due to this issue.

function commit(
    string[] calldata hopChains,
    string[] calldata hopAssets,
    string[] calldata hopAddresses,
    string calldata dstChain,
    string calldata dstAsset,
    string calldata dstAddress,
    string calldata srcAsset,
    address srcReceiver,
    uint48 timelock
) external payable returns (bytes32 Id) {
    if (msg.value == 0) revert FundsNotSent(); // Ensure funds are sent.
    if (timelock < block.timestamp) revert NotFutureTimelock(); // Ensure timelock is in the future.
    unchecked {
        ++contractNonce; // Increment nonce for uniqueness.
    }
    Id = bytes32(blockHashAsUint ^ contractNonce);
 
    // Store HTLC details.
    contracts[Id] = HTLC(
        msg.value,
        bytes32(bytes1(0x01)),
        uint256(1),
        payable(msg.sender),
        payable(srcReceiver),
        timelock,
        uint8(1)
    );

function commit(
    string[] calldata hopChains,
    string[] calldata hopAssets,
    string[] calldata hopAddresses,
    string calldata dstChain,
    string calldata dstAsset,
    string calldata dstAddress,
    string calldata srcAsset,
    address srcReceiver,
    uint48 timelock,
    uint256 amount,
    address tokenContract
) external returns (bytes32 Id) {
    if (amount == 0) revert FundsNotSent(); // Ensure funds are sent.
    if (timelock < block.timestamp) revert NotFutureTimelock(); // Ensure timelock is in the future.
    IERC20 token = IERC20(tokenContract);
 
    if (token.balanceOf(msg.sender) < amount) revert InsufficientBalance();
    if (token.allowance(msg.sender, address(this)) < amount)
        revert NoAllowance();
    token.safeTransferFrom(msg.sender, address(this), amount);
 
    unchecked {
        ++contractNonce; // Increment nonce for uniqueness.
    }
    Id = bytes32(blockHashAsUint ^ contractNonce);
 
    // Store HTLC details.
    contracts[Id] = HTLC(
        amount,
        bytes32(bytes1(0x01)),
        uint256(1),
        tokenContract,
        timelock,
        uint8(1),
        payable(msg.sender),
        payable(srcReceiver)
    );

Reverting in the commit() function whenever an ID exists will break the commit() function because contractNonce cannot increase anymore, as ID = bytes32(blockHashAsUint ^ contractNonce).

Therefore, consider placing the Id calculation in a loop that increments contractNonce until a non-existent Id is found.

In the source chain, the user have a total control on the timelock he will set for his funds. This allow him to steal from the LP by using the refund function while still redeeming on the destination chain, here are the steps of how the attack would go :

Steps to Exploit

1. User initiates a cross-chain swap:
   • The user calls commit on the source chain.
   • The LP locks funds on the destination chain by calling lock.
2. User sets a minimal timelock:
   • The user calls addLock on the source chain with:
     • A timelock set to block.timestamp + 1.
     • The generated hashlock.
3. User front-runs LP's redeem attempt:
   • On the next block, the LP attempts to redeem the funds.
   • The user front-runs this transaction and calls refund:
     • Since the timelock (block.timestamp + 1) has already passed, the refund succeeds.
   • The user obtains the secret from the hashlock in the process.
4. User redeems on the destination chain:
   • Using the secret, the user calls redeem on the destination chain before the LP's timelock expires (usually set to ~15 minutes).
   • The user successfully retrieves the funds on the destination chain.
5. Optional exploitation with Atomic Pool:
   • If the LP uses an atomic pool, the user can additionally call slash to penalize the LP further by slashing its stake. Consequences : the attacker get his refund on source chain AND get the fund on destination chain, while slashing the LP stacked fund in the process too.

function addLock(
    bytes32 Id,
    bytes32 hashlock,
    uint48 timelock
) external _exists(Id) returns (bytes32) {
    HTLC storage htlc = contracts[Id];
    if (htlc.claimed == 2 || htlc.claimed == 3) revert AlreadyClaimed();
    if (timelock < block.timestamp) revert NotFutureTimelock();
    if (msg.sender == htlc.sender) {
        if (htlc.hashlock == bytes32(bytes1(0x01))) {
            htlc.hashlock = hashlock;
            htlc.timelock = timelock;
        } else {
            revert HashlockAlreadySet(); // Prevent overwriting hashlock.
        }
        emit TokenLockAdded(Id, hashlock, timelock);
        return Id;
    } else {
        revert NoAllowance(); // Ensure only allowed accounts can add a lock.
    }
}

Force a minimum timelock value in the addLock() function.

In the error message InvalidSigniture();, the word "Signature" is spelled incorrectly as "Signiture." It should be spelled "Signature."

Correct the typo.

-- revert InvalidSigniture(); // Ensure valid signature.
++ revert InvalidSignature(); // Ensure valid signature.

Custom Errors, available from Solidity compiler version 0.8.4, provide benefits such as smaller contract size, improved gas efficiency, and better protocol interoperability. Replace require statements with Custom Errors for a more streamlined and user-friendly experience. Furthermore, custom errors are much clearer as they allow for parameter values, making debugging much easier.

modifier _exists(bytes32 Id) {
    require(hasHTLC(Id), "HTLC Not Exists");
    _;
}

In the LayerswapV8ERC20 contract, the errors HTLCNotExists and TransferFailed on line (L53, L59) are declared but never used.

    /// @notice Amounts already withdrawn this period for each token.
    mapping(address => uint256) public rateLimitDuration;

If the error is redundant and there is no missing logic where it can be used, it should be removed.