Lines of code

Vulnerability details

Impact

Contract NotionalTradeModule.sol will not compile due to an error caused by missing import of safeMath and the directiveusing for.

Since safeMath is not imported and no using for directive, the contract would not even compile as it would throw an error on [line 677]
<https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol#L677&gt;

            preTradeSendTokenBalance.sub(currentSendTokenBalance),

The contract NotionalTradeModule.sol has a function called _updateSetTokenPositions which according to the comments was copied from thetrade module here

File: NotionalTradeModule.sol #L650-L680
<https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol#L650-L680&gt;

It’s important to note one of the comments in the function _updateSetTokenPositions fromNotionalTradeModule.sol

* @dev WARNING: This function is largely copied from the trade module

In this function,ie _updateSetTokenPositions , the return statement executes an arithmetic operation using safeMath sub function. However while coping majority of this function from the trade module thesafeMath library was never imported.

Note the return statement :

        return (
            preTradeSendTokenBalance.sub(currentSendTokenBalance),
            currentReceiveTokenBalance.sub(preTradeReceiveTokenBalance)
        );

To use safeMath we often use the using for directive as follows

using safeMath for uint256;

But the above directive is also not present on the contract NotionalTradeModule.sol.

#Proof of Concept

The NotionalTradeModule.sol contract only imports the following.
imports

import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { IERC777 } from "@openzeppelin/contracts/token/ERC777/IERC777.sol";
import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
import { ReentrancyGuard } from "@openzeppelin/contracts/utils/ReentrancyGuard.sol";
import { Address } from "@openzeppelin/contracts/utils/Address.sol";


import { IController } from "../../../interfaces/IController.sol";
import { IDebtIssuanceModule } from "../../../interfaces/IDebtIssuanceModule.sol";
import { IModuleIssuanceHook } from "../../../interfaces/IModuleIssuanceHook.sol";
import { IWrappedfCash, IWrappedfCashComplete } from "../../../interfaces/IWrappedFCash.sol";
import { IWrappedfCashFactory } from "../../../interfaces/IWrappedFCashFactory.sol";
import { ISetToken } from "../../../interfaces/ISetToken.sol";
import { ModuleBase } from "../../lib/ModuleBase.sol";

Available using for directive
The only available using for directive is the address as shown below.

    using Address for address;

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

Tools Used

Vim,Remix

Recommended Mitigation Steps

Importing the safeMath library from openzeppelin and add the using for directive as follows
using safeMath for uint256; should fix the issue

This bug is as a result of Copy and paste from other contracts therefore I would advice you to avoid copy pasting and if we must be careful when copying code from other contracts.

---

The text was updated successfully, but these errors were encountered:

All reactions