code-423n4 / 2022-06-notional-coop-findings

1 stars 1 forks source link

QA Report #48

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

1) LOW: Data Storage Allocation and Retrieval Can Have Mismatches

L29, wfCashBase.sol

L8, wfCashERC4626.sol

Proxies are recommended to be used without having a constructor due to the fact of how the data storage slots are allocated during deployment and that they can't be accessed after the deployment by the proxies. Although no further vulnerabilities where found regarding this topic, Openzeppelin suggests not using constructors while creating implementations.


2) LOW: No address zero check while deploying contracts

L29, wfCashBase.sol

L8, wfCashERC4626.sol

Contracts may need to be redeployed if any of the addresses required as input is the address(0) on the pointed lines of code. Adding checks to prevent that scenario may solve this potential issue.


3) LOW: Pseudo-DoS while Deploying via the Factory may waste others' gas

Currently the deployWrapper function on WrappedfCashFactory.sol does not check or implements a deployment cooldown to deploy another wfCash contract with a different maturity for the same currencyId. If user A frontruns other users while deploying, it may waste the legit deployers gas. Although deploying a wrapped contract does not grant any further privileges, this may frustrate other users who tried to deploy and waste their gas. This is not technically a denial of service because once the contract is deployed everyone will be able to interact with the maturity-id coin contract, but has the same principle that impedes others to deploy a contract.

def test_waste_others_gas(factory, env):
    markets = env.notional.getActiveMarkets(2)
    for times in range(10):
        #Using any valid maturity at most will pass the deployment requirements
        maturity = markets[0][1] - 86400 * times // 
        txn = factory.deployWrapper(2, maturity, {'from': env.whales["DAI_CONTRACT"]})