stake.link

Summary

There is no way to revoke the approval which given via the approvefunction
They may able execute transfers even after the owner revokes their permission using the setApprovalForAll function.

Vulnerability Details

The setApprovalForAll function allows the owner to approve anyone as the operator.

In the same vein, the approve function allows the owner or operator to approve anyone to transfer the lock.

Note that in the function, lock cannot be approved to the owner (but can be approved to any of the operators), and can be called by the owner/operator (see the isApprovedForAll modifier).

If the operator approves himself to the lock, using the approve function, and later on, his operator status gets revoked, his lock approval status is not cleared, meaning he still has access to the lock.

As an extreme example

• User1 owns 5 locks.

• He calls the setApprovalForAll setting User2 as his operator.

• User2 calls the approve function on all 5 locks (It succeeds as there's no check preventing this unlike with the lock owner), getting herself both operator approval and token approvals.

• User1 revokes User2's operator status.

• User2 still has access to the locks and can transfer them.

Impact

Uncleared approval, gives access to transfer token.

Tools Used

Manual code review

Recommendations

Include a check to see if the _to in the approve function is an operator, revert if it is.
Or clear an operator's token approvals after revoking his operator status.