code-423n4 / 2023-01-numoen-findings

0 stars 0 forks source link

Unsecured Token Transfer in SwapHelper Contract via Insecure SafeTransferLib Method. #220

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-numoen/blob/2ad9a73d793ea23a25a381faadc86ae0c8cb5913/src/periphery/SwapHelper.sol#L42

Vulnerability details

Impact

SafeTransferLib.safeTransfer(tokenIn, msg.sender, amount0Delta > 0 ? uint256(amount0Delta) : uint256(amount1Delta));

If the SafeTransferLib contract is not secure or safe, the tokens sent using the SwapHelper.swap function may be vulnerable to malicious attacks such as theft, unauthorized transfers, or manipulation of token balances, it may also be exploited through the SwapHelper.swap function, potentially leading to serious consequences such as loss of funds or manipulation of token balances.

Proof of Concept

The vulnerability here lies in the fact that the contract blindly relies on the SafeTransferLib contract to send tokens, without checking if the contract is secure or safe. An attacker could potentially exploit this by replacing the SafeTransferLib contract with their own malicious version.

  1. An attacker creates a malicious contract that has the same interface as the SafeTransferLib contract.
  2. The attacker sets the address of the SafeTransferLib contract to the address of their malicious contract.
  3. The attacker invokes the swap function, causing it to use the malicious SafeTransferLib contract.
  4. The malicious SafeTransferLib contract could then steal the tokens being sent or manipulate the outcome in any other way.

Example.

pragma solidity ^0.8.4;

contract MaliciousSafeTransferLib {
function safeTransfer(address token, address to, uint256 amount) public {
// steal the tokens being sent
address attacker = msg.sender;
attacker.transfer(amount);
}
}```
## Tools Used

Manual audit, Vs Code

## Recommended Mitigation Steps

1. Verify the code of the `SafeTransferLib` contract to ensure it is secure and does not have any known vulnerabilities.

2. If any vulnerabilities are found, implement a fix for them.

3. If the `SafeTransferLib` contract is deemed to be insecure, replace it with a secure, audited contract or write your own implementation of a safe token transfer.

```solidity
function safeTransfer(address _to, uint256 _value) public {
    require(address(this).balance >= _value, "Insufficient funds");
    require(_to != address(0), "Invalid recipient");
    require(bytes(abi.encodePacked(_to)).length == 20, "Invalid recipient");

    (bool success, ) = _to.call.value(_value)("");
    require(success, "Transfer failed");
}

This code uses the require statement to ensure that the recipient is a valid Ethereum address, has a sufficient balance to cover the transfer, and that the transfer itself was successful. If any of these conditions are not met, the transfer will not be executed, and any funds will remain safe.

c4-judge commented 1 year ago

berndartmueller marked the issue as unsatisfactory: Invalid