The burn function checks on the first line whether the caller is either an approved spender or the owner of the token. It then proceeds to call the _removeTokenFrom function which only checks for the token owner.
Vulnerability Detail
A call of the withdraw function from an approved spender would end up with a reverted transaction. withdraw calls _burn, which performs the same _isApprovedOrOwner check. The _burn function calls _removeTokenFrom, which performs the following check:
assert(idToOwner[_tokenId] == _from);
This will always fail for an approved spender that is not the owner of the token. The usage of assert instead of require shows that this check was not meant to fail in a normal execution flow.
Impact
Low - approved spenders cannot call the merge and withdraw functions.
Code Snippet
N/A
Tool used
Manual Review
Recommendation
Either restrict merge and withdraw only to the token owner, or add a function parameter to the _burn function that would allow to bypass this check for these edge cases.
Funny Merlot Yeti
Low/Info
Logic issue in the _burn function of VotingEscrow
Summary
The
burnfunction checks on the first line whether the caller is either an approved spender or the owner of the token. It then proceeds to call the_removeTokenFromfunction which only checks for the token owner.Vulnerability Detail
A call of the
withdrawfunction from an approved spender would end up with a reverted transaction.withdrawcalls_burn, which performs the same_isApprovedOrOwnercheck. The_burnfunction calls_removeTokenFrom, which performs the following check:assert(idToOwner[_tokenId] == _from);This will always fail for an approved spender that is not the owner of the token. The usage of
assertinstead ofrequireshows that this check was not meant to fail in a normal execution flow.Impact
Low - approved spenders cannot call the
mergeandwithdrawfunctions.Code Snippet
N/A
Tool used
Manual Review
Recommendation
Either restrict
mergeandwithdrawonly to the token owner, or add a function parameter to the_burnfunction that would allow to bypass this check for these edge cases.