router: attacker can benefit from users borrowing allowance

[rajatbeladiya]

Affected smart contract:

BaseRouter.sol BorrowingVault.sol ConnextRouter.sol SimpleRouter.sol

Severity:

High

Impact:

users debt asset can be stolen and debtToken will be minted to user

Description:

Note: BaseRouter.sol inhereted to ConnextRouter.sol and SimpleRouter.sol so both contracts are vulnerable and here BaseRouter.sol considered as ConnextRouter.sol and SimpleRouter.sol

https://github.com/Fujicracy/fuji-v2/blob/50fd0b74ccee1a73a459118e50e044a2bcfacd10/packages/protocol/src/vaults/borrowing/BorrowingVault.sol#L221-L239

BorrowingVault.sol => here user can borrow if user self want to borrow or user give _spendBorrowAllowance to other user to borrow.

BaseRouter.sol => xBundle() internally called calls _bundleInternal() which takes actions[] and args[] from user and it is also can be used to borrow funds from BorrowingVault.sol.

if user execute borrow from this function user needs to give _spendBorrowAllowance to BaseRouter.sol.

https://github.com/Fujicracy/fuji-v2/blob/50fd0b74ccee1a73a459118e50e044a2bcfacd10/packages/protocol/src/abstracts/BaseRouter.sol#L157-L165

here Borrow Action calls BorrowingVault.sol’s borrow() function, where it is checked if (caller != owner) it will check _spendAllowance

https://github.com/Fujicracy/fuji-v2/blob/50fd0b74ccee1a73a459118e50e044a2bcfacd10/packages/protocol/src/vaults/borrowing/BorrowingVault.sol#L231-L233

in this case caller will be BaseRouter.sol and owner will be user

Suppose,

Alice gives MAX _spendBorrowAllowance to BaseRouter.sol and borrow 1 token using xBundle() and PayBack, still remaining _spendBorrowAllowance will be large.

So, Attacker can call xBundle() function with args as owner = address(Alice), receiver = address(Attacker), amount = (Alice’s max allowed borrow amount)

Borrow action will be executed and debtAsset will be transfer to Attacker and debtShare will be minted to Alice

Attacke scenario:

1. Alice gives _spendBorrowAllowance to MAX or larger than he actually borrows
2. Alice Borrow 1 token using xBundle() and PayBack => noDebt right now
3. Attacker can call xBundle() function with Borrow Action and args => receiver = address(attacker), owner = address(Alice) amount = (Alice’s max allowed borrow amount)
4. executed Borrow Action and attacker can get away with Alice’s borrowed amount

Recommended Mitigation Steps:

add check at BaseRouter.sol that msg.sender is owner or you need to change logic to borrow function of BorrowingVault.sol

[0xdcota]

Borrow allowance has two arguments in the state variable. The receiver of the borrowed amount, and the operator (an address that is allowed to execute the operation). In the described scenario, Alice will have to approve the "attacker" as the receiver, which would not make sense from a user point of view.

Refer to v0.0.1 reviewed during audit competition.

https://github.com/Fujicracy/fuji-v2/blob/1b939ec84af137db430fc2aa1b4c6f15e5254003/packages/protocol/src/vaults/VaultPermissions.sol#L33-L35

This issue is dismissed.