search

code-423n4 / 2022-07-swivel-findings

0 stars 1 forks source link

Functions of MarketPlace.sol will always revert #134

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-07-swivel/blob/main/VaultTracker/VaultTracker.sol#L7 https://github.com/code-423n4/2022-07-swivel/blob/main/Marketplace/MarketPlace.sol#L8 https://github.com/code-423n4/2022-07-swivel/blob/main/Creator/Creator.sol#L41

Vulnerability details

Impact

Functions which call VaultTracker admin functions (e.g. addNotional) from MarketPlace will always revert since the admin is Creator.

Proof of Concept

VaultTracker.sol has an authorized(admin) modifier which only allows admin to call these functions. And the Creator will create VaultTracker, so the immutable admin of VaultTracker will always be Creator. But some functions in MarketPlace will also call these admin-only functions of VaultTracker, leading to these functions will always revert.

These lines below will call admin functions of VaultTracker in Marketplace/MarketPlace.sol:

120:    if (!IVaultTracker(market.vaultTracker).addNotional(t, a)) { revert Exception(25, 0, 0, address(0), address(0)); }
251:    if (!IVaultTracker(market.vaultTracker).addNotional(n, a)) { revert Exception(25, 0, 0, address(0), address(0)); }

136:    if (!IVaultTracker(market.vaultTracker).removeNotional(t, a)) { revert Exception(26, 0, 0, address(0), address(0)); }
269:    if (!IVaultTracker(market.vaultTracker).removeNotional(n, a)) { revert Exception(26, 0, 0, address(0), address(0)); }

203:    uint256 interest = IVaultTracker(markets[p][u][m].vaultTracker).redeemInterest(t);

102:    IVaultTracker(market.vaultTracker).matureVault(exchangeRate);

302:    if (!IVaultTracker(markets[p][u][m].vaultTracker).transferNotionalFrom(f, t, a)) { revert Exception(27, 0, 0, address(0), address(0)); }
316:    if (!IVaultTracker(markets[p][u][m].vaultTracker).transferNotionalFrom(msg.sender, t, a)) { revert Exception(27, 0, 0, address(0), address(0)); }

329:    IVaultTracker(markets[p][u][m].vaultTracker).transferNotionalFee(f, a);

Tools Used

Manual Review

Recommended Mitigation Steps

Set admin when Creator create VaultTracker, and add admin argument in VaultTracker constructor:

constructor(..., address _admin, ...){
    admin = _admin;
    ...
}
JTraversa commented 2 years ago

Duplicate of #36

robrobbins commented 2 years ago

addressed: https://github.com/Swivel-Finance/gost/pull/411

bghughes commented 2 years ago

Duplicate of #36