search

code-423n4 / 2023-01-drips-findings

0 stars 2 forks source link

[M-03] Front-running possibility #214

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-drips/blob/main/src/NFTDriver.sol#L249 https://github.com/code-423n4/2023-01-drips/blob/main/src/NFTDriver.sol#L272 https://github.com/code-423n4/2023-01-drips/blob/main/src/AddressDriver.sol#L170

Vulnerability details

Impact

The methods approve, setApprovalForAll and _transferFromCaller in AddressDriver and NFTDriver can be monitored for transactions and front-ran. Imagine the following scenario:

  1. Bob holds some ERC20 tokens
  2. For some reason, NFTDriver decides to approve Bob to spend specific amount of tokens.
  3. Bob was expecting that and was already monitoring the mempool, so he front-runs the transaction with a transfer transaction to another address he controls
  4. Now his is able to spend more thant expected.

Proof of Concept

249: function approve(address to, uint256 tokenId) public override whenNotPaused {

289: erc20.safeApprove(address(dripsHub), type(uint256).max);
174: erc20.safeApprove(address(dripsHub), type(uint256).max);

Tools Used

Manual audit

Recommended Mitigation Steps

Use increase/decrease allowance instead.

GalloDaSballo commented 1 year ago

Have downgraded to NC per the rulebook

https://github.com/code-423n4/org/issues/51

GalloDaSballo commented 1 year ago

NC

c4-judge commented 1 year ago

GalloDaSballo changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

GalloDaSballo marked the issue as grade-c