`burn` and `mintTo` in `Basket.sol` vulnerable to reentrancy

[code423n4]

Handle

0xalpharush

Vulnerability details

Impact

The functions mintTo and burn make external calls prior to updating the state. If a basket contains an ERC777 token, attackers can mint free basket tokens.

Proof of Concept

An attacker could reenter the mintTo function when the contract pulls an ERC777 token from the user and mint more tokens than they deposited.

Tools Used

Slither

Recommended Mitigation Steps

Move external calls after state updates. It is best practice to make external calls after updating state in accordance with the check-effect-interact pattern.

[frank-beard]

For now we are only concerned with 'Defi Safe' tokens that conform to the erc-20 standard. It is expected that publishers and users should do due diligence when adding assets to a basket

[GalloDaSballo]

I agree with the warden finding that the function can be subject to re-entrancy.

Because the exploit is dependent on the token of choice, this is not a guarantee but rather the possibility of the system being vulnerable.

I highly recommend the sponsor to add re-entrancy checks.

As for the finding, because it is conditional on using a vulnerable token, I'll downgrade to medium severity as it requires an external condition for the re-entrance to be possible

[GalloDaSballo]

After re-review I agree with medium severity on reentrancy findings on burn and mint, in contract with the "generic" re-entrnacy finding with the additional bounties that lack any poc.

The findings with specific poc have been rated as separate findings (some of which have high risk) as they show an actual way to exploit the protocol