Beanstalk: The Finale

Summary

Detection of the reentrancy bug (https://github.com/crytic/not-so-smart-contracts/tree/master/reentrancy).

Vulnerability Details

External calls:
- _cancelPodOrder(podOrder,LibTransfer.To.INTERNAL) (contracts/beanstalk/market/MarketplaceFacet/Order.sol#60)
- returndata = address(token).functionCall(data) (node_modules/@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol#96)
- LibTransfer.sendToken(C.bean(),amountBeans,podOrder.orderer,mode) (contracts/beanstalk/market/MarketplaceFacet/Order.sol#137)
- token.safeTransfer(recipient,amount) (contracts/libraries/Token/LibTransfer.sol#71)
- (success,returndata) = target.call{value: value}(data) (node_modules/@openzeppelin/contracts/utils/Address.sol#87)
External calls sending eth:
- _cancelPodOrder(podOrder,LibTransfer.To.INTERNAL) (contracts/beanstalk/market/MarketplaceFacet/Order.sol#60)
- (success,returndata) = target.call{value: value}(data) (node_modules/@openzeppelin/contracts/utils/Address.sol#87)
State variables written after the call(s):
- s.sys.podOrders[id] = beanAmount (contracts/beanstalk/market/MarketplaceFacet/Order.sol#61)
ReentrancyGuard.s (contracts/beanstalk/ReentrancyGuard.sol#16) can be used in cross function reentrancies:
- PodTransfer.allowancePods(address,address,uint256) (contracts/beanstalk/market/MarketplaceFacet/PodTransfer.sol#35-41)

Impact

An attacker uses the re-entrancy bug in _cancelPodOrder to transfer more tokens when the user tries to execute the call.

Tools Used

Slither

Recommendations

Apply the check-effects-interactions pattern (https://docs.soliditylang.org/en/v0.4.21/security-considerations.html#re-entrancy) by using re-entrancy guards not already handled by LibTransfer.