Open code423n4 opened 2 years ago
_burn function,redeem,redeemToassets and after the transfer calls in wfcashlogic.sol
each event should use three indexed fields if there are there or more fields https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol#L78 https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol#L94
brownie docs says the cofig file should be called brownie-config.yaml https://eth-brownie.readthedocs.io/en/stable/config.html https://github.com/code-423n4/2022-06-notional-coop/blob/main/notional-wrapped-fcash/brownie-config.yml
https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol#L19
instances include: https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol#L19
mitigation: use =0.8.x and version but lock the version of solidity
code is little hard to read because of comments , remove the comments.
instances include: in wfcashbase.sol instead (IERC20 underlyingToken, /* */) = getUnderlyingToken(); instead (IERC20 assetToken, /* */, /* */) = getAssetToken(); instead : (token, /* */) = getUnderlyingToken() instead :(token, /* */, /* */) = getAssetToken(); use: (IERC20 underlyingToken) = getUnderlyingToken(); use:(IERC20 assetToken) = getAssetToken(); use: (token,,)=getAssetToken(); use: (token,,)=getUnderlyingToken()
(IERC20 underlyingToken, /* */) = getUnderlyingToken();
(IERC20 assetToken, /* */, /* */) = getAssetToken();
(token, /* */) = getUnderlyingToken()
(token, /* */, /* */) = getAssetToken();
(IERC20 underlyingToken) = getUnderlyingToken();
(IERC20 assetToken) = getAssetToken();
(token,,)=getAssetToken();
(token,,)=getUnderlyingToken()
should be: reentracy in wfcashLogic.sol:222 and those 3 redeem function make it :Updatable in notionalTradeModule.sol https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol#L114 https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol#L117
1. salt is supposed to be an arbitrary value not 0 make it an arbitrary value and if not arbitrary it can lead to replay attacks.
2. no emits after important functions
_burn function,redeem,redeemToassets and after the transfer calls in wfcashlogic.sol
3. Event is missing indexed fields
each event should use three indexed fields if there are there or more fields https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol#L78 https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol#L94
4. best practice to make brownie config file to make notional-wrapped fcash its .yml to .yaml
brownie docs says the cofig file should be called brownie-config.yaml https://eth-brownie.readthedocs.io/en/stable/config.html https://github.com/code-423n4/2022-06-notional-coop/blob/main/notional-wrapped-fcash/brownie-config.yml
5. using a old version of solidity can be prone to bugs and no overflow check
https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol#L19
6 . using ^0.8.0 you can use one version for deloyment and diffrent version for testing which can cause issues.
instances include: https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol#L19
mitigation: use =0.8.x and version but lock the version of solidity
7.
code is little hard to read because of comments , remove the comments.
instances include: in wfcashbase.sol instead
(IERC20 underlyingToken, /* */) = getUnderlyingToken();
instead(IERC20 assetToken, /* */, /* */) = getAssetToken();
instead :(token, /* */) = getUnderlyingToken()
instead :(token, /* */, /* */) = getAssetToken();
use:(IERC20 underlyingToken) = getUnderlyingToken();
use:(IERC20 assetToken) = getAssetToken();
use:(token,,)=getAssetToken();
use:(token,,)=getUnderlyingToken()
8 . typos
should be: reentracy
in wfcashLogic.sol:222 and those 3 redeem function
make it :Updatable in notionalTradeModule.sol https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol#L114 https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol#L117