Missing validation in MultiSigs ( + enable "clean compile" in bash script)

compile.sh

@@ -4,7 +4,11 @@
 set -x;
 # Shuttle `tasks/index.ts` file to temp file before cleaning/compiling starts
 mv -n ./tasks/index.ts ./tasks/temp_index.ts && touch ./tasks/index.ts;
-# Clean and compile all contracts.
+# Clean if necessary
+if [ "$1" == "clean" ]; then
+    hardhat clean;
+fi
+# Compile all contracts
 hardhat compile;
 # Shuttle `tasks/index.ts` file from temp file to orig location after cleaning/compiling ends
 # Run regardless of above line's failure or sucesss

contracts/multisigs/CharityApplications.sol

@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.16;
+
+import {Validator} from "../core/validator.sol";
 import "./CharityApplicationsStorage.sol";
 import {ICharityApplications} from "./interfaces/ICharityApplications.sol";
 import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
@@ -23,7 +25,7 @@ contract CharityApplications is MultiSigGeneric, StorageApplications, ICharityAp
    */
 
   modifier proposalExists(uint256 proposalId) {
-    require(proposals[proposalId].proposer != address(0), "Proposal dne");
+    require(Validator.addressChecker(proposals[proposalId].proposer), "Proposal dne");
     _;
   }
 
@@ -88,14 +90,20 @@ contract CharityApplications is MultiSigGeneric, StorageApplications, ICharityAp
     address _seedAsset,
     uint256 _seedAmount
   ) public override initializer {
-    super.initialize(owners, _approvalsRequired, _requireExecution, _transactionExpiry);
+    require(Validator.addressChecker(_accountsContract), "Invalid Accounts contract");
+    require(Validator.addressChecker(_seedAsset), "Invalid seed asset");
+    require(
+      _seedSplitToLiquid >= 0 && _seedSplitToLiquid <= 100,
+      "Seed split to liquid must be between 0 & 100"
+    );
     // set Applications Multisig storage items
     proposalCount = 1;
     config.accountsContract = _accountsContract;
     config.seedSplitToLiquid = _seedSplitToLiquid;
     config.gasAmount = _gasAmount;
     config.seedAsset = _seedAsset;
     config.seedAmount = _seedAmount;
+    super.initialize(owners, _approvalsRequired, _requireExecution, _transactionExpiry);
   }
 
   /**
@@ -210,7 +218,7 @@ contract CharityApplications is MultiSigGeneric, StorageApplications, ICharityAp
     if (config.gasAmount > 0) {
       // get the first member of the new endowment
       address payable signer = payable(proposals[proposalId].application.members[0]);
-      require(signer != address(0), "Endowment Member not set");
+      require(Validator.addressChecker(signer), "Endowment Member not set");
 
       // check matic balance on this contract
       if (address(this).balance >= config.gasAmount) {
@@ -270,8 +278,8 @@ contract CharityApplications is MultiSigGeneric, StorageApplications, ICharityAp
     address seedAsset,
     uint256 seedAmount
   ) public override ownerExists(msg.sender) {
-    require(seedAsset != address(0), "Seed Asset is not a valid address");
-    require(accountsContract != address(0), "Accounts Contract is not a valid address");
+    require(Validator.addressChecker(seedAsset), "Seed Asset is not a valid address");
+    require(Validator.addressChecker(accountsContract), "Accounts Contract is not a valid address");
     require(
       seedSplitToLiquid >= 0 && seedSplitToLiquid <= 100,
       "Seed split to liquid must be between 0 & 100"

contracts/multisigs/MultiSigGeneric.sol

@@ -1,9 +1,11 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.16;
 
+import {Validator} from "../core/validator.sol";
 import "./storage.sol";
 import {IMultiSigGeneric} from "./interfaces/IMultiSigGeneric.sol";
 import "@openzeppelin/contracts/utils/introspection/ERC165.sol";
+import "@openzeppelin/contracts/utils/Strings.sol";
 import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
 import {ReentrancyGuard} from "@openzeppelin/contracts/security/ReentrancyGuard.sol";
 import {Utils} from "../lib/utils.sol";
@@ -19,7 +21,7 @@ contract MultiSigGeneric is
    *  Modifiers
    */
   modifier onlyWallet() {
-    require(msg.sender == address(this));
+    require(msg.sender == address(this), "Can only be called by the MultiSig itself");
     _;
   }
 
@@ -34,19 +36,22 @@ contract MultiSigGeneric is
   }
 
   modifier transactionExists(uint256 transactionId) {
-    require(transactions[transactionId].destination != address(0), "Transaction dne");
+    require(Validator.addressChecker(transactions[transactionId].destination), "Transaction dne");
     _;
   }
 
   modifier confirmed(uint256 transactionId, address _owner) {
-    require(confirmations[transactionId].confirmationsByOwner[_owner], "Transaction is confirmed");
+    require(
+      confirmations[transactionId].confirmationsByOwner[_owner],
+      "Transaction is not confirmed"
+    );
     _;
   }
 
   modifier notConfirmed(uint256 transactionId, address _owner) {
     require(
       !confirmations[transactionId].confirmationsByOwner[_owner],
-      "Transaction is not confirmed"
+      "Transaction is already confirmed"
     );
     _;
   }
@@ -69,13 +74,11 @@ contract MultiSigGeneric is
     _;
   }
 
-  modifier notNull(address addr) {
-    require(addr != address(0), "Address cannot be a zero address");
-    _;
-  }
-
   modifier validApprovalsRequirement(uint256 _ownerCount, uint256 _approvalsRequired) {
-    require(_approvalsRequired <= _ownerCount && _approvalsRequired != 0);
+    require(
+      _approvalsRequired <= _ownerCount && _approvalsRequired != 0,
+      "Invalid approvals requirement"
+    );
     _;
   }
 
@@ -94,6 +97,10 @@ contract MultiSigGeneric is
   function addOwners(address[] memory owners) public virtual override onlyWallet {
     require(owners.length > 0, "Empty new owners list passed");
     for (uint256 o = 0; o < owners.length; o++) {
+      require(
+        Validator.addressChecker(owners[o]),
+        string.concat("Invalid owner address at index: ", Strings.toString(o))
+      );
       require(!isOwner[owners[o]], "New owner already exists");
       // increment active owners count by 1
       activeOwnersCount += 1;
@@ -131,6 +138,7 @@ contract MultiSigGeneric is
     address currOwner,
     address newOwner
   ) public virtual override onlyWallet ownerExists(currOwner) ownerDoesNotExist(newOwner) {
+    require(Validator.addressChecker(newOwner), "Invalid new owner address");
     isOwner[currOwner] = false;
     isOwner[newOwner] = true;
     emitOwnerReplaced(currOwner, newOwner);
@@ -309,7 +317,10 @@ contract MultiSigGeneric is
   ) internal initializer validApprovalsRequirement(owners.length, _approvalsRequired) {
     require(owners.length > 0, "Must pass at least one owner address");
     for (uint256 i = 0; i < owners.length; i++) {
-      require(!isOwner[owners[i]] && owners[i] != address(0));
+      require(
+        Validator.addressChecker(owners[i]),
+        string.concat("Invalid owner address at index: ", Strings.toString(i))
+      );
       isOwner[owners[i]] = true;
     }
     activeOwnersCount = owners.length;
@@ -332,7 +343,8 @@ contract MultiSigGeneric is
     uint256 value,
     bytes memory data,
     bytes memory metadata
-  ) internal virtual override notNull(destination) returns (uint256 transactionId) {
+  ) internal virtual override returns (uint256 transactionId) {
+    require(Validator.addressChecker(destination), "Invalid destination address");
     transactionId = transactionCount;
     transactions[transactionId] = MultiSigStorage.Transaction({
       destination: destination,

contracts/normalized_endowment/endowment-multisig/EndowmentMultiSig.sol

@@ -1,9 +1,11 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.16;
+
+import {Validator} from "../../core/validator.sol";
+import {Utils} from "../../lib/utils.sol";
 import {MultiSigGeneric} from "../../multisigs/MultiSigGeneric.sol";
-import {IEndowmentMultiSigEmitter} from "./interfaces/IEndowmentMultiSigEmitter.sol";
 import {MultiSigStorage} from "../../multisigs/storage.sol";
-import {Utils} from "../../lib/utils.sol";
+import {IEndowmentMultiSigEmitter} from "./interfaces/IEndowmentMultiSigEmitter.sol";
 
 /**
  * @notice the endowment multisig contract
@@ -31,7 +33,7 @@ contract EndowmentMultiSig is MultiSigGeneric {
     bool _requireExecution,
     uint256 _transactionExpiry
   ) public initializer {
-    require(_emitter != address(0), "Invalid Address");
+    require(Validator.addressChecker(_emitter), "Invalid Emitter Address");
     ENDOWMENT_ID = _endowmentId;
     EMITTER_ADDRESS = _emitter;
     super.initialize(_owners, _required, _requireExecution, _transactionExpiry);