search

hats-finance / Velvet-Capital-0x0bb0c08fd9eeaf190064f4c66f11d18182961f77

Core smart contracts of Velvet Capital
Other
0 stars 1 forks source link

DepositBatch : hardcoding the SWAP_TARGET address could not flexible when there is any upgrade or bug fixes #49

Open hats-bug-reporter[bot] opened 4 months ago

hats-bug-reporter[bot] commented 4 months ago

Github username: @aktech297 Twitter username: kaka Submission hash (on-chain): 0xf7a9552f39d546497d7603c421cc7c485b202694a8221c6492562ec4e76ecb09 Severity: low

Description: Description\

In the DepositBatch contract, the enso SWAP_TARGET address is hardcoded.

This may not be flexible in certain situations.

As we know , in the software development, continuous updates and upgrades are essential in software development to ensure security, efficiency, and functionality. In the context of smart contracts, particularly those handling swap router settings in DeFi, flexibility to update is crucial.

we saw these type of instances some other places, the hardcode contract address were used. This is not safe.

Impact\

Inability to Update Routers: If the swap router becomes outdated, inefficient, or compromised, the contract cannot switch to a new router without redeploying the entire contract.

Missed Opportunities for Optimization: The contract cannot take advantage of new or more efficient routers that may be developed after the contract is deployed.

Need for Redeployment: Any changes to the swap router settings would require redeploying the contract, which can be costly and time-consuming.

Migration Issues: Users and liquidity providers may need to migrate to a new contract if changes are necessary, which can be disruptive and may result in loss of user trust or liquidity.

  1. Proof of Concept (PoC) File

https://github.com/hats-finance/Velvet-Capital-0x0bb0c08fd9eeaf190064f4c66f11d18182961f77/blob/aa47c9ff85bcc2bede62978c3895668b549da125/contracts/bundle/DepositBatch.sol#L20-L23

contract DepositBatch is ReentrancyGuard {
  // The address of Enso's swap execution logic; swaps are delegated to this target.
  address constant SWAP_TARGET = 0x38147794FF247e5Fc179eDbAE6C37fff88f68C52;
  1. Revised Code File (Optional)

We suggest to implement separate functions to set these contract address when there is any change. These function should be called only by the admin/trusted party.

langnavina97 commented 4 months ago

We can redeploy the contract, this prevents us from using any other protocol that is not approved by the auditors. If we could update it, we could use any Solver solution without having to check with the security auditors. @aktech297

aktech297 commented 4 months ago

It's about redeploying the entire contract which would cost more gas. Whereas if there is separate admin controlled function, it would be easy and cheap.

As per severity classification this would fall under low. Pls check