Beanstalk: The Finale
Beanstalk
DeFiHardhatFoundry
250,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

`TractorFacet.tractor()` incorrectly works with functions containing `LibEth.refundEth()`

T1MOH

Summary

Conceptually Tractor is similar to Farm with additional functionality. Farm contains just multisig functionaluty to execute a bunch of delegate calls to Beanstalk. Tractor offers the same functionality, and allows to make calls on behalf of another user, i.e. adds authentification to those calls.

FarmFacet uses modifier withEth to not refund ETH in the middle of the batch:

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/df2dd129a878d16d4adc75049179ac0029d9a96b/protocol/contracts/libraries/Token/LibEth.sol#L14-L22
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/df2dd129a878d16d4adc75049179ac0029d9a96b/protocol/contracts/beanstalk/farm/FarmFacet.sol#L53-L62

// signals to Beanstalk functions that they should not refund Eth
// at the end of the function because the function is wrapped in a Farm function
modifier withEth() {
@> if (msg.value > 0) s.sys.isFarm = 2;
_;
if (msg.value > 0) {
s.sys.isFarm = 1;
LibEth.refundEth();
}
}
function refundEth() internal {
AppStorage storage s = LibAppStorage.diamondStorage();
@> if (address(this).balance > 0 && s.sys.isFarm != 2) {
(bool success, ) = msg.sender.call{value: address(this).balance}(new bytes(0));
require(success, "Eth transfer Failed.");
}
}

Problem is that such modifier is missing in TractorFacet.tractor(). It means Eth will be refunded in the middle of the batch, so problems arise when batch contains multiple functions using ETH balance.

Impact

Functions containing refund logic work incorrectly. They refund ETH immediately as such function is executed, instead of making it in the end of a batch.

Tractor module works incorrectly with following functions:

  • DepotFacet.advancedPipe()

  • DepotFacet.etherPipe()

  • TokenFacet.wrapEth()

Tools Used

Manual Review

Recommendations

Add modifier FarmFacet.withEth to function TractorFacet.tractor().

Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Refunding ETH to the caller can be exploited using the Tractor component to call any arbitrary function on behalf of the publisher

Appeal created

T1MOH Submitter
12 months ago
inallhonesty Lead Judge
12 months ago
giovannidisiena Auditor
12 months ago
inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.