Summary

The ApprovalFacet contract implements a setApprovalForAll function, which is intended to allow users to grant or revoke approval for an operator to manage all of their tokens. However, this function's effects are never utilized in any other part of the contract, rendering it completely non-functional.

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/8c8710df547f7d7c5dd82c5381eb6b34532e4484/protocol/contracts/beanstalk/silo/ApprovalFacet.sol#L189C3-L195C6

Proof Of Concept

1: Alice calls setApprovalForAll(Bob, true), believing she has granted Bob permission to manage all her tokens.

2: The contract emits an ApprovalForAll event, reinforcing Alice's belief.

3: Alice later calls setApprovalForAll(Bob, false), thinking she has revoked Bob's permissions.

4: In reality, neither action has any effect on Bob's ability to manage Alice's tokens, as no other function checks the approval status set by setApprovalForAll.

Impact

1: Users are provided with a false sense of security, believing they can grant or revoke broad permissions when in reality, these actions have no effect.

2: External systems relying on the ApprovalForAll event may make incorrect assumptions about the contract's state.

Tools Used

Manual review

Recommendations

1: Introduce a mechanism to revoke individual token allowances in addition to the global approval.