``setApprovalForAll`` Function Fails to Grant Approval
Report ID
#43424
Report Type
Smart Contract
Has PoC
Yes
Target
https://arbiscan.io/address/0xD1A0060ba708BC4BCD3DA6C37EFa8deDF015FB70
Impacts
Contract fails to deliver promised returns, but doesn't lose value
Details
In the ApprovalFacet contract of the Beanstalk protocol (using a Diamond proxy pattern), the setApprovalForAll function is intended to allow a user to authorize a spender to manage all of their ERC-1155 deposits. While the function emits the expected event and updates the internal mapping, the approval does not take effect in practice — the spender remains unable to interact with the user's deposits.
Vulnerability Details
Contract: ApprovalFacet (Diamond Proxy)
Functions Affected: setApprovalForAll(address spender, bool approved) and transferDeposits function
Issue:
The vulnerability lies in the setApprovalForAll function within the ApprovalFacet contract of Beanstalk. This function is meant to allow a user to grant blanket approval to another address (a spender) to manage all of their ERC-1155 deposits. Internally, the function sets s.accts[LibTractor._user()].isApprovedForAll[spender] = approved and emits the corresponding ApprovalForAll event.
However, this approval is not respected by other parts of the protocol — most notably, the Depot contract's transferDeposit function. Even after successfully calling setApprovalForAll, the spender is unable to move the user's deposits; the call to transferDeposit reverts.
This is demonstrated in the provided proof of concept (PoC), where a call to transferDeposit fails after setApprovalForAll, but succeeds when using approveDeposit instead. This suggests a disconnect between the approval state set in setApprovalForAll and the logic that checks permissions for deposit transfers.
As a result, the setApprovalForAll function appears to update internal state correctly but is functionally inert, leading to unexpected behavior and broken delegation flows across the protocol.
Impact Details
Contract fails to deliver promised returns, since users can't use the setApprovalForAll function to approve deposit transfers, despite the function appearing to work correctly.
Mitigation
Change the _spendDepositAllowance in the LibSilo contract to:
By changing this function, we'll start to check the isApprovedForAll variable when spending the allowance, different from before (now) where we are only checking the currentAllowance value.
Proof of Concept
Run with:
forge test --fork-url https://arb1.lava.build --match-test test_IssueSetApprovalsForAll -vvBIC Response
This is a known issue, therefore we are closing the report and no reward will be issued.