DeFiHardhatFoundry
250,000 USDC
View results
Submission Details
Severity: high
Invalid

Incorrect msg.sender in context of a blueprint execution can cause a loss of funds

Line of code

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/df2dd129a878d16d4adc75049179ac0029d9a96b/protocol/contracts/libraries/LibTractor.sol#L116

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/df2dd129a878d16d4adc75049179ac0029d9a96b/protocol/contracts/libraries/Token/LibEth.sol#L18

Summary

Incorrect msg.sender in context of a blueprint execution can cause a loss of funds

Vulnerability Details

When executing a blueprint the action is done on behalf of the publisher not necassarily the msg sender.

function _user() internal view returns (address payable user) {
user = _getActivePublisher();
if (uint160(bytes20(address(user))) <= 1) {
user = payable(msg.sender);
}
}

In the snippet above we get the address of the active publisher.

LibTractor._incrementBlueprintNonce(requisition.blueprintHash);
LibTractor._setPublisher(payable(requisition.blueprint.publisher));
_;
LibTractor._resetPublisher()

Above shows how the publisher is set for a tx. First we increment the blueprint noce and we set the publisher. Then the logic of the call is executed, and the publisher is then reset again.

The code will call this user function like so below

LibTractor._user(),

to get the real user address to make the code compatible with blue print execution.

However, when handing ETH / WETH / refund call, the msg.sender is still used instead of LibTractor._user()

In LibWETH.sol

function wrap(uint256 amount, LibTransfer.To mode) internal {
deposit(amount);
LibTransfer.sendToken(IERC20(WETH), amount, msg.sender, mode);
}
function unwrap(uint256 amount, LibTransfer.From mode) internal {
amount = LibTransfer.receiveToken(IERC20(WETH), amount, msg.sender, mode);
withdraw(amount);
(bool success, ) = msg.sender.call{value: amount}(new bytes(0));
require(success, "Weth: unwrap failed");
}

when handling refund:

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.");
}
}

this is a issue.

suppose a blueprint action involve some action, that involves convert token to ETH then relies on the refundETH() to send the ETH out to the LibTractor._user() => publisher, the ETH is actually sent to msg.sender instead of publisher.

The incorrect user has now received the eth.

Impact

In certain cases the incorrect user may receive another users funds. Loss of funds for the publisher when executing blueprint

Tools Used

Manual review

Recommendations

ensure the correct user is sent eth/ weth etc... When calling via blueprint execution

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year 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

giovannidisiena Auditor
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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