Fujicracy / fuji-v2

Cross-chain money market aggregator
https://fuji-v2-frontend.vercel.app
15 stars 10 forks source link

CS - BaseRouter insecure implementation of approvals and permits #199

Closed 0xdcota closed 1 year ago

0xdcota commented 1 year ago

Where: https://github.com/Fujicracy/fuji-v2/blob/bffa427797ad8d6df63671868ee8823574e044ca/packages/protocol/src/abstracts/BaseRouter.sol#L78

Description: The BaseRouter contract allows bundling multiple actions in one call to execute them in one transaction. Some of these actions (namely Deposit, Payback, Borrow, Withdraw) can abuse the permits and approvals given by the victim to the router. That approach is not an exploit per se as it is required to execute specific actions. For example, if a user wants to deposit via router, they must approve the router to transfer the assets. On the other hand, if a user wants to deposit and borrow in a bundle, they must approve the router to transfer the assets but also permit the router to borrow on the user's behalf. However, special care should be taken when handling bundles that include actions using approvals and permits. More information can be found in the Recommendation section. Additionally, the protocol supports unlimited approvals that increases the risk. It is worth mentioning that there are two cases of approvals:

  1. withdrawal/borrow approvals that are in control of Fujidao Labs developers, and
  2. token (e.g. USDC) approvals that are out of control of Fujidao Labs developers.

Recommendations: • Require the receiver to be the owner of tokens (sender).

0xdcota commented 1 year ago

Potential attacks have been addressed, but other forking tests broke and need to be fixed. Self note to review:

forge test --mp test/forking/goerli/ConnextRouter.t.sol --mt "test_bridgeInbound" ✔️ resolved Dec-27-2022

forge test --mp test/forking/optimism-goerli/ConnextRouter.t.sol --mt "test_bridgeInbound" ✔️ resolved Dec-27-2022

forge test --mp test/forking/polygon/SimpleRouter.t.sol --mt "test_closePositionWithFlashloan" ✔️ resolved Dec-22-2022

forge test --mp test/forking/polygon/SimpleRouter.t.sol --mt "test_debtSwap" ✔️ resolved Dec-22-2022