Beanstalk: The Finale

Summary

Low likelihood DOS when the transfers of full amount of some supported tokens are being executed, due to the current transfer logic.

Vulnerability Details

Take a look at https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/8c8710df547f7d7c5dd82c5381eb6b34532e4484/protocol/contracts/libraries/Token/LibTokenApprove.sol#L36-L42

This function is what gets finally called in the case there is any transfer, that's not via transfer() but from transferFrom(), issue however is that, Beanstalk is to support all ERC2o tokens as stated in the readMe, issue however is that tokens exist that do not accept an approval to the 0 value, see https://github.com/d-xo/weird-erc20?tab=readme-ov-file#revert-on-zero-value-approvals.

Now see the instances where spendAllowance gets queried accross protocol:

This then means that when using such ERC-20 tokens, the transfers would always revert when the transfer that's to be done is that of all the allowance that sender has given the user, leading to a DOS on the attempt at transfer.

NB: The hinted instances where spendAllowance gets queried are also helper functions, so this bug case also affects the external transfer attempts that gets routed via this in core functionalities across protocol.

Impact

DOS on the attempt at transfer for some users as they can't transfer out all their approved tokens.

Imo, impact -medium (just a Dos and +1 approval would sort it out, but could be of higher impact if the +1 is of high value $wise ), likelihood- low, so severity -> low.

Tools Used

• Manual review

• Juancito's guide on multichain deployments: https://github.com/0xJuancito/multichain-auditor?tab=readme-ov-file#hardcoded-contract-addresses

Recommendations

Consider outrightly not supporting these tokens, alternatively heavily doxument this factor and have users approve +1 wei of these sort of tokens (this would naturally work if the token is of a high decimal, however if it's a low decimal, then +1 could be worth too much for a sender to approve to the receiver as an extra, so safe bet is just to outrightly not support these tokens).