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

1 stars 1 forks source link

QA Report #146

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

1. Unused private function _safeNegInt88() in wfCashERC4626

Impact

In wfCashERC4626 contract, function _safeNegInt88() is declared but never use. And it’s a private function, so no one can call it outside the contract either.

Proof Of Concept

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashERC4626.sol#L243

Recommended Mitigation Steps

Remove _safeNegInt88() function.

2. Call totalSupply() 2 times to get already cached value.

Impact

In wfCachERC4626.convertToShares(), line 53 has already cached totalSupply but line 60 call totalSupply() again instead of using supply variable directly. It’s inconsistent when compare to convertToAssets() function which use supply variable.

Proof Of Concept

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashERC4626.sol#L60

Recommended Mitigation Steps

Use supply instead of totalSupply()

3. Implementation is not align with documentation

Detail

Function NotionalTradeModule._mintFCashPosition() is used to mint fCash position, but the natspec docs said that this function is used to redeem fCash position

Proof of concept

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol#L418

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol#L415

4. Inconsistent use of floating pragma and fixed solidiy version

Impact

Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively.

https://swcregistry.io/docs/SWC-103

In wrapped fcash, it uses ^0.8.0 and 0.8.11 inconsistently

Occurences

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashERC4626.sol#L2

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/lib/DateTime.sol#L2

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/lib/Constants.sol#L2

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/lib/Types.sol#L2

Recommended Mitigation Steps

Use fixed solidity version