search

code-423n4 / 2021-07-spartan-findings

0 stars 0 forks source link

Max approvals are risky #100

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

0xRajeev

Vulnerability details

Impact

Giving max approval (total supply) for DAO's SPARTA/tokens to ROUTER is risky because if ROUTER contract gets compromised then DAO's SPARTA/token balance can be drained.

Proof of Concept

https://github.com/code-423n4/2021-07-spartan/blob/e2555aab44d9760fdd640df9095b7235b70f035e/contracts/Dao.sol#L260

https://github.com/code-423n4/2021-07-spartan/blob/e2555aab44d9760fdd640df9095b7235b70f035e/contracts/Dao.sol#L269

Tools Used

Manual Analysis

Recommended Mitigation Steps

Use increaseAllowance instead of approve to increase allowance only as much as required for the transfer being made. This also addresses the potential race-condition susceptibility of approve().

ghoul-sol commented 3 years ago

This is industry wide practice, while not the best, I don't see specific vector attack described so it's a best practice recommendation hence non-critical.