search

code-423n4 / 2021-12-amun-findings

0 stars 0 forks source link

Setting allowance to uint256(-1) is bad practice #63

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

itsmeSTYJ

Vulnerability details

Impact

The countless defi hacks have shown that the reason why some of these contracts were exploited is because they:

  1. gave other core contracts unlimited token allowance
  2. users gave the contracts unlimited token allowance

Proof of Concept

Even though it is more expensive, it is IMO always better to give just enough allowance to execute the transferFrom. A quick ctrl + f search for uint256(-1) will show you which contracts should be updated.

0xleastwood commented 2 years ago

This is a UX prioritisation. It doesn't make sense to reapprove on each call as there are no tokens in the contract. Will make this non-critical.