sherlock-audit / 2023-10-aloe-judging

9 stars 6 forks source link

mstpr-brainbot - Race condition in lender shares #121

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

mstpr-brainbot

medium

Race condition in lender shares

Summary

The tokenized share for lenders has a race condition issue. Upon examining the code, it's evident that couriers have an allowance to deposit on behalf of users. Malicious couriers can potentially frontrun the approval transaction and deposit double the amount on behalf of the users. Moreover, the token does not adhere to the ERC4626-ERC20 standard, as it lacks the increaseAllowance and decreaseAllowance methods.

Vulnerability Detail

Suppose Alice approves Bob for 100 1e18 Lender share tokens using the approve method inside the Lender contract. Later, Alice realizes she actually needed to approve 200 1e18 tokens to Bob, so she initiates another transaction with the adjusted approval amount. Bob, acting quickly, spends the initial 100 1e18 tokens. However, by the time Alice's second transaction is mined, Bob still has an allowance of 200 1e18.

Here more details on the race condition attack: https://docs.google.com/document/d/1YLPtQxZu1UAvO9cZ1O2RPXBbT0mooh4DYKjA_jp-RLM/edit#heading=h.m9fhqynw2xvt

Impact

The Lender contracts are not erc4626-erc20 standard and the front-running is exists for p2p lender token approvals

Code Snippet

https://github.com/sherlock-audit/2023-10-aloe/blob/main/aloe-ii/core/src/Lender.sol#L321-L327

Tool used

Manual Review

Recommendation

Add the increase-decrease allowance methods from ERC20

sherlock-admin2 commented 1 year ago

2 comment(s) were left on this issue during the judging contest.

tsvetanovv commented:

Low

MohammedRizwan commented:

low severity. openzeppelin has removed the increaseAllowance() and decreaseAllowance() functions.