La audit fixes (#116)

- [x] close #90 - [x] close #91 - [x] close #92 - [x] close #93 - [x] close #95 - [x] close #96 - [x] close #97 - [x] close #98 - [x] close #119 --------- Co-authored-by: Alberto Granzotto <[email protected]>
NeokingdomDAO · Oct 20, 2023 · b5e785b · b5e785b
1 parent 4e4c782
commit b5e785b
Show file tree

Hide file tree

Showing 45 changed files with 235 additions and 97 deletions.
diff --git a/contracts/GovernanceToken/GovernanceToken.sol b/contracts/GovernanceToken/GovernanceToken.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.16;
+pragma solidity 0.8.16;
 
 import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
 import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";
@@ -46,6 +46,10 @@ contract GovernanceToken is Initializable, HasRole, GovernanceTokenSnapshot {
         string memory name,
         string memory symbol
     ) public initializer {
+        require(
+            address(roles) != address(0),
+            "GovernanceToken: 0x0 not allowed"
+        );
         _initialize(name, symbol);
         _setRoles(roles);
     }
@@ -154,6 +158,15 @@ contract GovernanceToken is Initializable, HasRole, GovernanceTokenSnapshot {
         uint256 amount
     ) public virtual onlyRole(Roles.RESOLUTION_ROLE) {
         _mint(to, amount);
+
+        if (
+            _shareholderRegistry.isAtLeast(
+                _shareholderRegistry.CONTRIBUTOR_STATUS(),
+                to
+            )
+        ) {
+            _redemptionController.afterMint(to, amount);
+        }
     }
 
     /**
@@ -292,16 +305,6 @@ contract GovernanceToken is Initializable, HasRole, GovernanceTokenSnapshot {
         super._afterTokenTransfer(from, to, amount);
         _voting.afterTokenTransfer(from, to, amount);
 
-        if (
-            from == address(0) &&
-            _shareholderRegistry.isAtLeast(
-                _shareholderRegistry.CONTRIBUTOR_STATUS(),
-                to
-            )
-        ) {
-            _redemptionController.afterMint(to, amount);
-        }
-
         // Invariants
         require(
             balanceOf(from) >= _vestingBalance[from],
@@ -342,7 +345,12 @@ contract GovernanceToken is Initializable, HasRole, GovernanceTokenSnapshot {
      * @param amount Amount of external tokens to wrap.
      */
     function _wrap(address from, uint amount) internal virtual {
-        tokenExternal.transferFrom(from, address(this), amount);
+        require(
+            tokenExternal.transferFrom(from, address(this), amount),
+            "GovernanceToken: transfer failed"
+        );
+        require(amount > 0, "GovernanceToken: attempt to wrap 0 tokens");
+
         uint256 settlementTimestamp = block.timestamp + settlementPeriod;
         depositedTokens[from].push(
             DepositedTokens(amount, settlementTimestamp)

diff --git a/contracts/GovernanceToken/GovernanceTokenBase.sol b/contracts/GovernanceToken/GovernanceTokenBase.sol
@@ -1,6 +1,5 @@
 // SPDX-License-Identifier: MIT
-
-pragma solidity ^0.8.16;
+pragma solidity 0.8.16;
 
 import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";
 import "../RedemptionController/IRedemptionController.sol";
@@ -69,7 +68,10 @@ abstract contract GovernanceTokenBase is ERC20Upgradeable, IGovernanceToken {
     //}
 
     function _unwrap(address from, address to, uint amount) internal virtual {
-        tokenExternal.transfer(to, amount);
+        require(
+            tokenExternal.transfer(to, amount),
+            "GovernanceToken: transfer failed"
+        );
         super._burn(from, amount);
     }
 

diff --git a/contracts/GovernanceToken/GovernanceTokenSnapshot.sol b/contracts/GovernanceToken/GovernanceTokenSnapshot.sol
@@ -1,6 +1,5 @@
 // SPDX-License-Identifier: MIT
-
-pragma solidity ^0.8.16;
+pragma solidity 0.8.16;
 
 import "@openzeppelin/contracts/utils/Arrays.sol";
 import "./IGovernanceToken.sol";

diff --git a/contracts/GovernanceToken/IGovernanceToken.sol b/contracts/GovernanceToken/IGovernanceToken.sol
@@ -1,6 +1,5 @@
 // SPDX-License-Identifier: MIT
-
-pragma solidity ^0.8.16;
+pragma solidity 0.8.16;
 
 import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol";
 import "../extensions/ISnapshot.sol";

diff --git a/contracts/InternalMarket/InternalMarket.sol b/contracts/InternalMarket/InternalMarket.sol
@@ -1,6 +1,5 @@
 // SPDX-License-Identifier: MIT
-
-pragma solidity ^0.8.16;
+pragma solidity 0.8.16;
 
 import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
 import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
@@ -23,13 +22,18 @@ contract InternalMarket is Initializable, HasRole, InternalMarketBase {
     /**
      * @dev Initializes the contract with the given roles and internal token.
      * @param roles DAORoles instance containing custom access control roles.
-     * @param tokenInternal_ Reference to governance token.
+     * @param governanceToken Reference to governance token.
      */
     function initialize(
         DAORoles roles,
-        IGovernanceToken tokenInternal_
+        IGovernanceToken governanceToken
     ) public initializer {
-        _initialize(tokenInternal_, 7 days);
+        require(
+            address(roles) != address(0) &&
+                address(governanceToken) != address(0),
+            "InternalMarket: 0x0 not allowed"
+        );
+        _initialize(governanceToken, 7 days);
         _setRoles(roles);
     }
 

diff --git a/contracts/InternalMarket/InternalMarketBase.sol b/contracts/InternalMarket/InternalMarketBase.sol
@@ -1,6 +1,5 @@
 // SPDX-License-Identifier: MIT
-
-pragma solidity ^0.8.16;
+pragma solidity 0.8.16;
 
 import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
 import "../ShareholderRegistry/IShareholderRegistry.sol";

diff --git a/contracts/NeokingdomToken/INeokingdomToken.sol b/contracts/NeokingdomToken/INeokingdomToken.sol
@@ -1,6 +1,5 @@
 // SPDX-License-Identifier: MIT
-
-pragma solidity ^0.8.16;
+pragma solidity 0.8.16;
 
 import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 

diff --git a/contracts/NeokingdomToken/NeokingdomToken.sol b/contracts/NeokingdomToken/NeokingdomToken.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.9;
+pragma solidity 0.8.16;
 
 import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
 import "@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol";

diff --git a/contracts/RedemptionController/IRedemptionController.sol b/contracts/RedemptionController/IRedemptionController.sol
@@ -1,6 +1,5 @@
 // SPDX-License-Identifier: MIT
-
-pragma solidity ^0.8.16;
+pragma solidity 0.8.16;
 
 import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
 import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol";

diff --git a/contracts/RedemptionController/RedemptionController.sol b/contracts/RedemptionController/RedemptionController.sol
@@ -1,6 +1,5 @@
 // SPDX-License-Identifier: MIT
-
-pragma solidity ^0.8.16;
+pragma solidity 0.8.16;
 
 import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
 import "./RedemptionControllerBase.sol";
@@ -36,6 +35,10 @@ contract RedemptionController is
      * @param roles The addresses of DAORoles for this contract.
      */
     function initialize(DAORoles roles) public initializer {
+        require(
+            address(roles) != address(0),
+            "RedemptionController: 0x0 not allowed"
+        );
         _setRoles(roles);
         _initialize();
     }

diff --git a/contracts/RedemptionController/RedemptionControllerBase.sol b/contracts/RedemptionController/RedemptionControllerBase.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.16;
+pragma solidity 0.8.16;
 
 import "./IRedemptionController.sol";
 

diff --git a/contracts/ResolutionManager/ResolutionManager.sol b/contracts/ResolutionManager/ResolutionManager.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.16;
+pragma solidity 0.8.16;
 
 import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
 import { Roles } from "../extensions/Roles.sol";
@@ -26,6 +26,13 @@ contract ResolutionManager is Initializable, ResolutionManagerBase, HasRole {
         IGovernanceToken governanceToken,
         IVoting voting
     ) public initializer {
+        require(
+            address(roles) != address(0) &&
+                address(shareholderRegistry) != address(0) &&
+                address(governanceToken) != address(0) &&
+                address(voting) != address(0),
+            "ResolutionManager: 0x0 not allowed"
+        );
         _setRoles(roles);
         _initialize(shareholderRegistry, governanceToken, voting);
     }
@@ -167,13 +174,6 @@ contract ResolutionManager is Initializable, ResolutionManagerBase, HasRole {
      * @param resolutionId The id of the resolution to approve.
      */
     function approveResolution(uint256 resolutionId) external virtual {
-        require(
-            _shareholderRegistry.isAtLeast(
-                _shareholderRegistry.MANAGING_BOARD_STATUS(),
-                _msgSender()
-            ),
-            "Resolution: only managing board can approve"
-        );
         _approveResolution(resolutionId);
     }
 
@@ -182,13 +182,6 @@ contract ResolutionManager is Initializable, ResolutionManagerBase, HasRole {
      * @param resolutionId The id of the resolution to reject.
      */
     function rejectResolution(uint256 resolutionId) external virtual {
-        require(
-            _shareholderRegistry.isAtLeast(
-                _shareholderRegistry.MANAGING_BOARD_STATUS(),
-                _msgSender()
-            ),
-            "Resolution: only managing board can reject"
-        );
         _rejectResolution(resolutionId);
     }
 
@@ -209,13 +202,6 @@ contract ResolutionManager is Initializable, ResolutionManagerBase, HasRole {
         address[] memory executionTo,
         bytes[] memory executionData
     ) external virtual {
-        require(
-            _shareholderRegistry.isAtLeast(
-                _shareholderRegistry.MANAGING_BOARD_STATUS(),
-                _msgSender()
-            ),
-            "Resolution: only managing board can update"
-        );
         _updateResolution(
             resolutionId,
             dataURI,

diff --git a/contracts/ResolutionManager/ResolutionManagerBase.sol b/contracts/ResolutionManager/ResolutionManagerBase.sol
@@ -1,6 +1,5 @@
 // SPDX-License-Identifier: MIT
-
-pragma solidity ^0.8.16;
+pragma solidity 0.8.16;
 
 import "../ShareholderRegistry/IShareholderRegistry.sol";
 import "../GovernanceToken/IGovernanceToken.sol";
@@ -267,7 +266,7 @@ abstract contract ResolutionManagerBase {
         bool isNegative,
         address[] memory executionTo,
         bytes[] memory executionData
-    ) internal virtual onlyPending(resolutionId) {
+    ) internal virtual onlyPending(resolutionId) exists(resolutionId) {
         emit ResolutionUpdated(msg.sender, resolutionId);
 
         Resolution storage resolution = resolutions[resolutionId];

diff --git a/contracts/ShareholderRegistry/IShareholderRegistry.sol b/contracts/ShareholderRegistry/IShareholderRegistry.sol
@@ -1,6 +1,5 @@
 // SPDX-License-Identifier: MIT
-
-pragma solidity ^0.8.16;
+pragma solidity 0.8.16;
 
 import "../extensions/ISnapshot.sol";
 

diff --git a/contracts/ShareholderRegistry/ShareholderRegistry.sol b/contracts/ShareholderRegistry/ShareholderRegistry.sol
@@ -1,6 +1,5 @@
 // SPDX-License-Identifier: MIT
-
-pragma solidity ^0.8.16;
+pragma solidity 0.8.16;
 
 import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
 import "./ShareholderRegistrySnapshot.sol";
@@ -29,6 +28,10 @@ contract ShareholderRegistry is
         string memory name,
         string memory symbol
     ) public initializer {
+        require(
+            address(roles) != address(0),
+            "ShareholderRegistry: 0x0 not allowed"
+        );
         _initialize(name, symbol);
         _setRoles(roles);
     }

diff --git a/contracts/ShareholderRegistry/ShareholderRegistryBase.sol b/contracts/ShareholderRegistry/ShareholderRegistryBase.sol
@@ -1,11 +1,8 @@
 // SPDX-License-Identifier: MIT
-
-// TODO: update _statuses when account has no shares
-// TODO: check who can move shares
-
-pragma solidity ^0.8.16;
+pragma solidity 0.8.16;
 
 import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";
+import "@openzeppelin/contracts/utils/Address.sol";
 import "../Voting/IVoting.sol";
 
 contract ShareholderRegistryBase is ERC20Upgradeable {
@@ -40,6 +37,10 @@ contract ShareholderRegistryBase is ERC20Upgradeable {
     }
 
     function _setStatus(bytes32 status, address account) internal virtual {
+        require(
+            !Address.isContract(account),
+            "ShareholderRegistry: cannot set status for smart contract"
+        );
         require(
             status == 0 || isAtLeast(SHAREHOLDER_STATUS, account),
             "ShareholderRegistry: address has no tokens"
@@ -83,7 +84,10 @@ contract ShareholderRegistryBase is ERC20Upgradeable {
     ) internal view virtual returns (bool) {
         return
             balance > 0 &&
-            // shareholder < investor < contributor < managing board
+            // investor < contributor < managing board
+            // TODO: shareholder is currently equivalent to investor.
+            // We need to verify with the lawyer whether we can remove it
+            // completely from the smart contracts.
             (status == INVESTOR_STATUS ||
                 status == SHAREHOLDER_STATUS ||
                 status == accountStatus ||

diff --git a/contracts/ShareholderRegistry/ShareholderRegistrySnapshot.sol b/contracts/ShareholderRegistry/ShareholderRegistrySnapshot.sol
@@ -1,7 +1,5 @@
 // SPDX-License-Identifier: MIT
-// OpenZeppelin Contracts v4.4.1 (token/ERC20/extensions/ERC20Snapshot.sol)
-
-pragma solidity ^0.8.16;
+pragma solidity 0.8.16;
 
 import "./ShareholderRegistryBase.sol";
 import "../extensions/Snapshottable.sol";

diff --git a/contracts/Voting/IVoting.sol b/contracts/Voting/IVoting.sol
@@ -1,6 +1,5 @@
 // SPDX-License-Identifier: MIT
-
-pragma solidity ^0.8.16;
+pragma solidity 0.8.16;
 
 import "../extensions/ISnapshot.sol";
 

diff --git a/contracts/Voting/Voting.sol b/contracts/Voting/Voting.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.16;
+pragma solidity 0.8.16;
 
 import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
 import "../ShareholderRegistry/IShareholderRegistry.sol";
@@ -18,6 +18,7 @@ contract Voting is VotingSnapshot, Initializable, HasRole {
      * @param roles Instance of a DAORoles contract.
      */
     function initialize(DAORoles roles) public initializer {
+        require(address(roles) != address(0), "Voting: 0x0 not allowed");
         _setRoles(roles);
     }
 
@@ -95,7 +96,7 @@ contract Voting is VotingSnapshot, Initializable, HasRole {
 
     /**
      * @notice Hook called on every governance token transfer.
-     * @dev Only the governance token can call this method.
+     * @dev Called by GovernanceToken and ShareholderRegistry.
      * @param from The sender's address.
      * @param to The receiver's address.
      * @param amount The amount transferred.

diff --git a/contracts/Voting/VotingBase.sol b/contracts/Voting/VotingBase.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.16;
+pragma solidity 0.8.16;
 
 import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol";
 import "../ShareholderRegistry/IShareholderRegistry.sol";

diff --git a/contracts/Voting/VotingSnapshot.sol b/contracts/Voting/VotingSnapshot.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.16;
+pragma solidity 0.8.16;
 
 import "@openzeppelin/contracts/utils/Arrays.sol";
 import "../extensions/Snapshottable.sol";

diff --git a/contracts/extensions/DAORoles.sol b/contracts/extensions/DAORoles.sol
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: MIT
 
-pragma solidity ^0.8.0;
+pragma solidity 0.8.16;
 
 import "@openzeppelin/contracts/access/AccessControl.sol";