Skip to content

Latest commit

 

History

History
258 lines (197 loc) · 10.9 KB

072.md

File metadata and controls

258 lines (197 loc) · 10.9 KB

Energetic Opaque Elephant

High

Malicious Actor Can Cause Integration Failures and Potential Financial Loss Due to Missing decimals() Function in PEAS Contract

Summary

The PEAS contract is missing the decimals() function, a de facto standard for ERC-20 tokens. This non-compliance with ERC-20 conventions can lead to integration failures with other contracts and tools, potentially causing financial loss for users interacting with the PEAS token. While tests are currently using a MockPEAS contract to work around this issue, the vulnerability remains in the production PEAS contract and must be addressed.

Root Cause

The decimals() function, although not strictly required by the IERC20 interface, is a widely adopted convention for ERC-20 tokens. The PEAS contract implementation omits this function, leading to non-compliance and potential integration issues.

Internal Pre-conditions

  1. The PEAS contract is deployed without the decimals() function.
  2. Other contracts or tools attempt to interact with the PEAS contract, expecting the decimals() function to be present.
  3. Developers or users are unaware of the missing decimals() function in the PEAS contract.

External Pre-conditions

  1. A decentralized exchange (DEX) or other DeFi protocol attempts to integrate the PEAS token.
  2. A wallet or other tool attempts to display or handle PEAS token balances.

Attack Path

  1. User (Alice) attempts to trade PEAS on a DEX: Alice connects her wallet to a DEX that has integrated PEAS.
  2. DEX calls PEAS.decimals(): The DEX attempts to call the decimals() function on the PEAS contract to display token balances and calculate exchange rates correctly.
  3. PEAS.decimals() reverts: Because the decimals() function is missing, the call reverts.
  4. DEX integration fails: The DEX integration with PEAS fails. The DEX may display an error message, prevent trading of PEAS, or display incorrect information.
  5. Alice cannot trade PEAS: Alice is unable to trade PEAS on the DEX. She may lose trading opportunities or be unable to access her PEAS tokens through the DEX.

Impact

Alice is unable to trade PEAS on the DEX. She suffers a loss of opportunity and potential financial loss if she cannot access or manage her PEAS tokens. The DEX also suffers reputational damage and potential loss of users due to the integration failure. Other users may experience similar issues with different integrations.

PoC

To demonstrate the vulnerability, I initially attempted to run tests against the deployed PEAS contract at address 0x02f92800F57BCD74066F5709F1Daa1A4302Df875. This resulted in a revert error when the contract attempted to call the missing decimals() function, as shown in the attached image

Image

To resolve this issue for testing purposes and demonstrate a working scenario, I implemented a MockPEAS contract. This mock contract includes the necessary decimals() function, allowing tests to proceed without the revert error. The MockPEAS contract is shown below:

pragma solidity ^0.8.0;

import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "../contracts/interfaces/IPEAS.sol";

contract MockPEAS is IPEAS, ERC20 {
    uint8 public _decimals; // Store decimals as a state variable

    constructor(string memory _name, string memory _symbol, uint8 _decimalsValue) ERC20(_name, _symbol) {
        _mint(msg.sender, 10_000_000 * 10 ** _decimalsValue); // Mint to the deployer for testing
        _decimals = _decimalsValue; // Initialize decimals
    }

    function burn(uint256 _amount) external virtual override {
        _burn(msg.sender, _amount); // Burn from the test contract (msg.sender)
        emit Burn(msg.sender, _amount);
    }

    function decimals() public view virtual override returns (uint8) {
        return _decimals; // Return the stored decimals value
    }

    // Add a mint function for testing purposes:
    function mint(address _to, uint256 _amount) public {
        _mint(_to, _amount);
    }

    // Add a setDecimals function to allow changing the decimals value for testing:
    function setDecimals(uint8 _newDecimals) public {
        _decimals = _newDecimals;
    }

    // ... other functions as needed for your tests ...
}

//Other Mock contracts

contract MockERC20 {
    string public name;
    string public symbol;
    uint8 public decimals;
    mapping(address => uint256) public balanceOf;
    mapping(address => mapping(address => uint256)) public allowance;

    constructor(string memory _name, string memory _symbol, uint8 _decimals) {
        name = _name;
        symbol = _symbol;
        decimals = _decimals;
    }

    function transfer(address recipient, uint256 amount) public returns (bool) {
        balanceOf[msg.sender] -= amount;
        balanceOf[recipient] += amount;
        return true;
    }

    function approve(address spender, uint256 amount) public returns (bool) {
        allowance[msg.sender][spender] = amount;
        return true;
    }

    // ... Add other mocked functions (like decimals, transferFrom, etc.) as required ...

    function mint(address to, uint256 amount) public {
        balanceOf[to] += amount;
    }
}


interface IUniswapV2Router02 {
    function WETH() external view returns (address);
}

contract MockUniswapV2Router is IUniswapV2Router02 {
    address private _weth;

    constructor(address wethAddress) {
        _weth = wethAddress;
    }

    function WETH() external view override returns (address) {
        return _weth;
    }
}

I then updated the test setup in WeightedIndexTest.sol to use the MockPEAS contract instead of the originally deployed PEAS contract. The relevant changes to the setUp() function are shown below:

contract WeightedIndexTest is PodHelperTest {
    //PEAS public peas;
    RewardsWhitelist public rewardsWhitelist;
    V3TwapUtilities public twapUtils;
    UniswapDexAdapter public dexAdapter;
    WeightedIndex public pod;
    MockFlashMintRecipient public flashMintRecipient;

    MockERC20 public dai; // Use MockERC20 for DAI
    MockUniswapV2Router public mockV2Router;
    MockERC20 public mockWeth;
    MockPEAS public peas; // Use MockPEAS
    
    

    address public peasAddress;
    address public mockDAI; // Address of the deployed mock DAI
    //address public dai = 0x6B175474E89094C44Da98b954EedeAC495271d0F;
    uint256 public bondAmt = 1e18;
    uint16 fee = 100;
    uint256 public bondAmtAfterFee = bondAmt - (bondAmt * fee) / 10000;
    uint256 public feeAmtOnly1 = (bondAmt * fee) / 10000;
    uint256 public feeAmtOnly2 = (bondAmtAfterFee * fee) / 10000;

    // Test users
    address public alice = address(0x1);
    address public bob = address(0x2);
    address public carol = address(0x3);

    event FlashMint(address indexed executor, address indexed recipient, uint256 amount);

    event AddLiquidity(address indexed user, uint256 idxLPTokens, uint256 pairedLPTokens);

    event RemoveLiquidity(address indexed user, uint256 lpTokens);

    function setUp() public override {
        super.setUp();
        //peas = PEAS(0x02f92800F57BCD74066F5709F1Daa1A4302Df875);
        peas = new MockPEAS("PEAS", "PEAS", 18);
        peasAddress = address(peas);
        twapUtils = new V3TwapUtilities();
        rewardsWhitelist = new RewardsWhitelist();

        dai = new MockERC20("MockDAI", "mDAI", 18);
        mockDAI = address(dai); // Store the address
        mockWeth = new MockERC20("Wrapped Ether", "WETH", 18);
        mockV2Router = new MockUniswapV2Router(address(mockWeth));

        dexAdapter = new UniswapDexAdapter(
            twapUtils,
            address(mockV2Router),  
            //0x7a250d5630B4cF539739dF2C5dAcb4c659F2488D, // Uniswap V2 Router
            0x68b3465833fb72A70ecDF485E0e4C7bD8665Fc45, // Uniswap SwapRouter02
            false
        );
        IDecentralizedIndex.Config memory _c;
        IDecentralizedIndex.Fees memory _f;
        _f.bond = fee;
        _f.debond = fee;
        address[] memory _t = new address[](1);
        _t[0] = address(peas);
        uint256[] memory _w = new uint256[](1);
        _w[0] = 100;
        address _pod = _createPod(
            "Test",
            "pTEST",
            _c,
            _f,
            _t,
            _w,
            address(0),
            false,
            abi.encode(
                mockDAI,
                //dai,
                peasAddress,
                //address(peas),
                mockDAI,
                //0x6B175474E89094C44Da98b954EedeAC495271d0F,
                0x7d544DD34ABbE24C8832db27820Ff53C151e949b,
                rewardsWhitelist,
                0x024ff47D552cB222b265D68C7aeB26E586D5229D,
                dexAdapter
            )
        );
        pod = WeightedIndex(payable(_pod));

        flashMintRecipient = new MockFlashMintRecipient();

        // Initial token setup for test users
        deal(address(peas), address(this), bondAmt * 100);
        deal(address(peas), alice, bondAmt * 100);
        deal(address(peas), bob, bondAmt * 100);
        deal(address(peas), carol, bondAmt * 100);
        deal(mockDAI, address(this), 5 * 10e18);

        // Approve tokens for all test users
        vm.startPrank(alice);
        peas.approve(address(pod), type(uint256).max);
        vm.stopPrank();

        vm.startPrank(bob);
        peas.approve(address(pod), type(uint256).max);
        vm.stopPrank();

        vm.startPrank(carol);
        peas.approve(address(pod), type(uint256).max);
        vm.stopPrank();
    }

With these changes, running the test suite, specifically tests that interact with the PEAS contract (e.g., test_bond), now executes successfully. The logs show that the decimals() function being called is MockPEAS::decimals(), and it returns the expected value (18). This demonstrates that the absence of the decimals() function in the original PEAS contract is the root cause of the integration issue. The successful test execution with MockPEAS confirms the vulnerability and its impact. The attached image shows the successful execution of the test with the MockPEAS contract in place.

Image

Mitigation

  1. Implement decimals() in PEAS Contract: Add the following function to the PEAS contract: function decimals() public view virtual override returns (uint8) { return 18; // Or the correct number of decimals for your token }
  2. Recompile and Redeploy: Recompile the PEAS contract and redeploy it to the appropriate environment.
  3. Update Integrations (If Necessary): If any contracts or tools have already integrated with the faulty PEAS contract, they may need to be updated to account for the newly added decimals() function.