prePO contest - csanuragjain's results

Lines of code

https://github.com/code-423n4/2022-03-prepo/blob/main/contracts/core/PrePOMarketFactory.sol#L42

Vulnerability details

Impacted Function:

createMarket

Description:

1. Owner calls createMarket with _tokenNameSuffix S1 and _tokenSymbolSuffix S2 which creates a new market M1 with _deployedMarkets[_salt] pointing to M1. Here salt can be S which is computed using _tokenNameSuffix and _tokenSymbolSuffix
2. This market is now being used
3. After some time owner again mistakenly calls createMarket with _tokenNameSuffix S1 and _tokenSymbolSuffix S2
4. Instead of returning error mentioning that this name and symbol already exists, new market gets created. The problem here is that salt which is computed using _tokenNameSuffix and _tokenSymbolSuffix will again come as S (as in step 1) which means _deployedMarkets[_salt] will now get updated to M2. This means reference to M1 is gone

Recommendation:

Add below check:

require(_deployedMarkets[_salt]==address(0), "Market already exists");

Deposit can surpass Max Cap

https://github.com/code-423n4/2022-03-prepo/blob/main/contracts/core/CollateralDepositRecord.sol#L67 https://github.com/code-423n4/2022-03-prepo/blob/main/contracts/core/CollateralDepositRecord.sol#L58

Function : setAccountDepositCap/setGlobalDepositCap

Problem: The deposits could surpass the max cap in case either setAccountDepositCap or setGlobalDepositCap is called with lower value than old one

Recommendation: Add a check on setGlobalDepositCap to see require(_globalDepositCap < _newGlobalDepositCap && _newGlobalDepositCap!=0) otherwise the existing deposit could already be surpassing _newGlobalDepositCap. Similarly add check on setAccountDepositCap to verify require(_accountDepositCap < _newAccountDepositCap && _newAccountDepositCap!=0)

No way to unblock accounts:

https://github.com/code-423n4/2022-03-prepo/blob/main/contracts/core/AccountAccessController.sol#L50

Function: blockAccounts

Problem: Once account is Blocked, there is no way to unblock it until and unless you call clearBlockedAccounts function which increments _blockedAccountsIndex. But this will unblock all accounts and not just the one we wished to unblock

Recommendation: Add a function which allows to unblock an account

  _blockedAccounts[_blockedAccountsIndex][_accounts[_i]] = false;

Zero address check missing

https://github.com/code-423n4/2022-03-prepo/blob/main/contracts/core/AccountAccessController.sol#L95

Function : setRoot

Description: Zero address checks on _newRoot variable is missing.

Recommendation: Consider adding require(_newRoot != address(0), "Invalid root");