Skip to content

Commit

Permalink
Audit fixes - Quantstamp 2 (#60)
Browse files Browse the repository at this point in the history
* fix: normalize dividends withdrawn so far, for both parties involved when transferring entity tokens [NAY-2]

* fix: double-counting of dividend payouts [NAY-3]

* fix: incorrect accounting of sysAdmins when reassigning system role [NAY-6]

* fix: fuzzer config

* fix: Invalid Entity Update [NAY-7]

* chore: remove unnecessary test file T02Helpers.sol

* fix: Id Aliasing Between Addresses and Associated Objectid [NAY-8]

* chore: delete T01SmartDeploymentV1.t.sol

* fix: Insufficient Contract Pausability [NAY-12]

* fix: Adding Wrapped Participation Tokens as Supported External [NAY-14]

* fix: phased diamond cut now keccak256 hashes all 3 diamond cut params to check if an upgrade is valid [NAY-1]

* fix: premium amount overspending [NAY-4]

* fix: Missing Input Validation (#61) [NAY-9]

* fix: Insufficient Contract Pausability [NAY-12]

* fix: Adding Wrapped Participation Tokens as Supported External [NAY-14]

* fix(6): trading commission total BP

* fix(3): stakeholders arrays are sized properly

* fix(8): validate upgrade expiration period updates

* fix(9): validate upgrade cancellation

* fix(10): validate token name is not empty

* fix: formatting

* fix(5): minimum policy coverage time

---------

Co-authored-by: Kevin Park <kevin@fruitful.gg>

* fix: Allowance Double-Spend Exploit [NAY-16]

* fix: owner and system admin are now mutually exclusive. removed optimistic matching in the match making algorithm. updated Nayms diamond and deployment scripts. [NAY-11]

* fix: improve observability [NAY-22]

* Adherence to Best Practices (#62)

* fix: unused imports, typos and redundant code

* fix: commission arrays length limit

* fix: index address in events

* fix: reuse variables

* fix: remove redundant struct

* chore: remove LibMeta.sol

* refactor: LibDiamond.initializeDiamondCut() throws error InitializationFunctionReverted instead of require msg

* docs: remove todo comments in code

* docs: improve explanation of UserFacet

* doc: fix some missing natspec comments

* doc: fix some typos

* fix: rename role updated event

* doc: clarify premium commission basis points

* fix: comment typos

* fix: policy event test

---------

Co-authored-by: Kevin Park <kevin@fruitful.gg>

* test: fix changePrank fails with no prank in progress to stop

* fix: duplicate function selector, missing externalWithdrawFromEntity in lock and _unlockAllFundTransferFunctions() amendment [NAY-12]

* fix: lock and unlockFunction() now emit FunctionsLocked and FunctionsUnlocked amendment [NAY-22]

* fix: remove _addressToBytes32(), _bytes32ToString(), unused lib imports, add docs to _stringToBytes32() [NAY-bonus1]

* chore: update version flag to post audit

---------

Co-authored-by: Kevin Park <kevin@fruitful.gg>
  • Loading branch information
amarinkovic and kevin-fruitful committed May 8, 2023
1 parent 9de0e39 commit eec4700
Show file tree
Hide file tree
Showing 66 changed files with 1,460 additions and 1,080 deletions.
1 change: 1 addition & 0 deletions .env.example
@@ -1,6 +1,7 @@
export LOCAL_RPC_URL=
export ETH_MAINNET_RPC_URL=
export ETH_GOERLI_RPC_URL=
export ETH_SEPOLIA_RPC_URL=
export ETH_MAINNET_API_KEY=
export ETH_GOERLI_API_KEY=
export ETHERSCAN_API_KEY=
Expand Down
2 changes: 2 additions & 0 deletions .gitignore
Expand Up @@ -32,6 +32,8 @@ facetsdeployed.txt
.vscode/targets.log
.vscode/launch.json

.history

bin/
.idea/.gitignore
.idea/contracts-next.iml
Expand Down
34 changes: 19 additions & 15 deletions Makefile
Expand Up @@ -35,8 +35,11 @@ gen-i: ## generate solidity interfaces from facet implementations
-s "run(string memory, string memory)" src/diamonds/nayms/interfaces/ 0.8.13 \
--ffi

prep-build: ## prepare buld, generate LibGeneratedNaymsFacetHelpers
node ./cli-tools/prep-build.js
prep-build: ## prepare buld, generate LibGeneratedNaymsFacetHelpers. This excludes ACL and Governance facets, which are deployed with the Nayms diamond.
node ./cli-tools/prep-build.js ACL Governance

prep-build-all: ## prepare buld, generate LibGeneratedNaymsFacetHelpers. This includes all facets in the src/diamonds/nayms/facets folder.
node ./cli-tools/prep-build.js

build: ## forge build
forge build --names --sizes
Expand Down Expand Up @@ -162,6 +165,8 @@ initNewDiamond=false
facetAction=1
senderAddress=0x931c3aC09202650148Edb2316e97815f904CF4fa
deploymentSalt=0xdeffffffff
owner=0x931c3aC09202650148Edb2316e97815f904CF4fa
systemAdmin=0x2dF0a6dB2F0eF1269bE777C856A7665eeC00649f

schedule-upgrade-goerli: ## schedule upgrade to goerli diamond, then upgrade
@forge script SmartDeploy \
Expand All @@ -179,7 +184,7 @@ schedule-upgrade-goerli: ## schedule upgrade to goerli diamond, then upgrade

deploy: ## smart deploy to goerli
@forge script SmartDeploy \
-s "smartDeploy(bool, bool, uint8, string[] memory, bytes32)" ${newDiamond} ${initNewDiamond} ${facetAction} ${facetsToCutIn} ${deploymentSalt} \
-s "smartDeploy(bool, address, address, bool, uint8, string[] memory, bytes32)" ${newDiamond} ${owner} ${systemAdmin} ${initNewDiamond} ${facetAction} ${facetsToCutIn} ${deploymentSalt} \
-f ${ETH_GOERLI_RPC_URL} \
--chain-id 5 \
--etherscan-api-key ${ETHERSCAN_API_KEY} \
Expand All @@ -194,7 +199,7 @@ deploy: ## smart deploy to goerli

deploy-mainnet: ## smart deploy to mainnet
@forge script SmartDeploy \
-s "smartDeploy(bool, bool, uint8, string[] memory, bytes32)" ${newDiamond} ${initNewDiamond} ${facetAction} ${facetsToCutIn} ${deploymentSalt} \
-s "smartDeploy(bool, address, address, bool, uint8, string[] memory, bytes32)" ${newDiamond} ${owner} ${systemAdmin} ${initNewDiamond} ${facetAction} ${facetsToCutIn} ${deploymentSalt} \
-f ${ETH_MAINNET_RPC_URL} \
--chain-id 1 \
--etherscan-api-key ${ETHERSCAN_API_KEY} \
Expand All @@ -205,12 +210,13 @@ deploy-mainnet: ## smart deploy to mainnet
-vv \
--ffi \
--broadcast \
--slow \
--verify --delay 30 --retries 10 \
; node cli-tools/postproc-broadcasts.js

deploy-mainnet-sim: ## simulate deploy to mainnet
@forge script SmartDeploy \
-s "smartDeploy(bool, bool, uint8, string[] memory, bytes32)" ${newDiamond} ${initNewDiamond} ${facetAction} ${facetsToCutIn} ${deploymentSalt} \
-s "smartDeploy(bool, address, address, bool, uint8, string[] memory, bytes32)" ${newDiamond} ${owner} ${systemAdmin} ${initNewDiamond} ${facetAction} ${facetsToCutIn} ${deploymentSalt} \
-f ${ETH_MAINNET_RPC_URL} \
--chain-id 1 \
--etherscan-api-key ${ETHERSCAN_API_KEY} \
Expand All @@ -220,7 +226,7 @@ deploy-mainnet-sim: ## simulate deploy to mainnet

deploy-sim: ## simulate smart deploy to goerli
forge script SmartDeploy \
-s "smartDeploy(bool, bool, uint8, string[] memory, bytes32)" ${newDiamond} ${initNewDiamond} ${facetAction} ${facetsToCutIn} ${deploymentSalt} \
-s "smartDeploy(bool, address, address, bool, uint8, string[] memory, bytes32)" ${newDiamond} ${owner} ${systemAdmin} ${initNewDiamond} ${facetAction} ${facetsToCutIn} ${deploymentSalt} \
-f ${ETH_GOERLI_RPC_URL} \
--chain-id 5 \
--etherscan-api-key ${ETHERSCAN_API_KEY} \
Expand All @@ -239,7 +245,7 @@ anvil-fork: ## fork goerli locally with anvil

anvil-deploy: ## smart deploy locally to anvil
forge script SmartDeploy \
-s "smartDeploy(bool, bool, uint8, string[] memory, bytes32)" true true 0 ${facetsToCutIn} ${deploymentSalt} \
-s "smartDeploy(bool, address, address, bool, uint8, string[] memory, bytes32)" true ${owner} ${systemAdmin} true 0 ${facetsToCutIn} ${deploymentSalt} \
-f http:\\127.0.0.1:8545 \
--chain-id 31337 \
--sender ${senderAddress} \
Expand All @@ -251,7 +257,7 @@ anvil-deploy: ## smart deploy locally to anvil

anvil-upgrade: ## smart deploy locally to anvil
forge script SmartDeploy \
-s "smartDeploy(bool, bool, uint8, string[] memory, bytes32)" false false 1 ${facetsToCutIn} ${deploymentSalt}\
-s "smartDeploy(bool, address, address, bool, uint8, string[] memory, bytes32)" false ${owner} ${systemAdmin} false 1 ${facetsToCutIn} ${deploymentSalt} \
-f http:\\127.0.0.1:8545 \
--chain-id 31337 \
--sender ${senderAddress} \
Expand Down Expand Up @@ -351,29 +357,27 @@ slither: ## run slither static analysis

upgrade-hash-goerli: ## generate upgrade hash
@forge script SmartDeploy \
-s "hash(bool, uint8, string[] memory, bytes32)" false 1 "[]" ${deploymentSalt} \
-s "hash(bool, address, address, bool, uint8, string[] memory, bytes32)" false ${owner} ${systemAdmin} ${initNewDiamond} 1 "[]" ${deploymentSalt} \
--fork-url ${ETH_GOERLI_RPC_URL} \
--chain-id 5 \
--etherscan-api-key ${ETHERSCAN_API_KEY} \
--ffi \
--silent \
--json \
| jq --raw-output '.returns.upgradeHash.value, .returns.cut.value'
&& jq --raw-output '.returns.upgradeHash.value, .returns.cut.value' broadcast/SmartDeploy.s.sol/5/dry-run/hash-latest.json

upgrade-hash-mainnet: ## generate upgrade hash
@forge script SmartDeploy \
-s "hash(bool, uint8, string[] memory, bytes32)" false 1 "[]" ${deploymentSalt} \
-s "hash(bool, address, address, bool, uint8, string[] memory, bytes32)" false ${owner} ${systemAdmin} ${initNewDiamond} 1 "[]" ${deploymentSalt} \
--fork-url ${ETH_MAINNET_RPC_URL} \
--chain-id 1 \
--etherscan-api-key ${ETHERSCAN_API_KEY} \
--ffi \
--silent \
--json \
| jq --raw-output '.returns.upgradeHash.value, .returns.cut.value'
&& jq --raw-output '.returns.upgradeHash.value, .returns.cut.value' broadcast/SmartDeploy.s.sol/1/dry-run/hash-latest.json

upgrade-hash-anvil: ## generate upgrade hash
forge script SmartDeploy \
-s "hash(bool, uint8, string[] memory, bytes32)" ${newDiamond} ${facetAction} ${facetsToCutIn} ${deploymentSalt} \
-s "hash(bool, address, address, bool, uint8, string[] memory, bytes32)" ${newDiamond} ${owner} ${systemAdmin} ${initNewDiamond} ${facetAction} ${facetsToCutIn} ${deploymentSalt} \
--sender ${senderAddress} \
--mnemonic-paths ./nayms_mnemonic.txt \
--mnemonic-indexes 19 \
Expand Down
154 changes: 107 additions & 47 deletions cli-tools/prep-build.js
@@ -1,65 +1,106 @@
#!/usr/bin/env node
const path = require('path')
const fs = require('fs')
const glob = require('glob')
const chalk = require('chalk')
const path = require("path");
const fs = require("fs");
const glob = require("glob");
const chalk = require("chalk");

const PROJECT_DIR = path.join(__dirname, "..");
const FACETS_SRC_DIR = path.join(
PROJECT_DIR,
"src",
"diamonds",
"nayms",
"facets"
);
const INTERFACES_SRC_DIR = path.join(
PROJECT_DIR,
"src",
"diamonds",
"nayms",
"interfaces"
);
const FACETS_DEPLOYED_TXT = path.join(PROJECT_DIR, "facetsdeployed.txt");
const GENERATED_DEPLOY_NAMES_SOL = path.join(
PROJECT_DIR,
"script",
"utils",
"LibGeneratedNaymsFacetHelpers.sol"
);

function filterFacetsToExclude(excludeList) {
const allFacets = glob
.sync("*Facet.sol", { cwd: FACETS_SRC_DIR })
.map((a) => path.basename(a).substring(0, a.indexOf("Facet")));
return allFacets.filter((facet) => !excludeList.includes(facet));
}

const PROJECT_DIR = path.join(__dirname, '..')
const FACETS_SRC_DIR = path.join(PROJECT_DIR, 'src', 'diamonds', 'nayms', 'facets')
const INTERFACES_SRC_DIR = path.join(PROJECT_DIR, 'src', 'diamonds', 'nayms', 'interfaces')
const FACETS_DEPLOYED_TXT = path.join(PROJECT_DIR, 'facetsdeployed.txt')
const GENERATED_DEPLOY_NAMES_SOL = path.join(PROJECT_DIR, 'script', 'utils', 'LibGeneratedNaymsFacetHelpers.sol')
// Parse command line arguments
const args = process.argv.slice(2);
const excludedFacets = args.map((arg) => arg.trim());

// load facets
const facetNames = glob.sync('*Facet.sol', { cwd: FACETS_SRC_DIR }).map(a => path.basename(a).substring(0, a.indexOf('Facet')))
const facetNames = filterFacetsToExclude(excludedFacets);

// load interfaces and methods
const facetData = {}
const REGEX = /^\s+function ([A-Za-z0-9_]+)\(/igm
facetNames.forEach(f => {
const facetData = {};
const REGEX = /^\s+function ([A-Za-z0-9_]+)\(/gim;
facetNames.forEach((f) => {
try {
const interfaceName = `I${f}Facet`
const src = fs.readFileSync(path.join(INTERFACES_SRC_DIR, `${interfaceName}.sol`)).toString('utf-8')
const methods = []
let m
const interfaceName = `I${f}Facet`;
const src = fs
.readFileSync(path.join(INTERFACES_SRC_DIR, `${interfaceName}.sol`))
.toString("utf-8");
const methods = [];
let m;
while ((m = REGEX.exec(src))) {
methods.push(m[1])
methods.push(m[1]);
}
if (!methods.length) {
throw new Error(`Empty interface`)
throw new Error(`Empty interface`);
}
facetData[f] = {
interfaceName,
methods,
}
};
} catch (err) {
console.error(chalk.red(`Error loading interface for facet ${f}`))
throw err
console.error(chalk.red(`Error loading interface for facet ${f}`));
throw err;
}
})
});

console.log(chalk.blue('== Facets =='))
console.log(chalk.blue("== Facets =="));
Object.entries(facetData).forEach(([facetName, data]) => {
console.log(`${facetName} - ${data.methods.length} methods`)
})
fs.writeFileSync(FACETS_DEPLOYED_TXT, facetNames.join("\n"), { encoding: 'utf-8'})
console.log(`${facetName} - ${data.methods.length} methods`);
});
fs.writeFileSync(FACETS_DEPLOYED_TXT, facetNames.join("\n"), {
encoding: "utf-8",
});

// write metadata solidity file
fs.writeFileSync(GENERATED_DEPLOY_NAMES_SOL, `
fs.writeFileSync(
GENERATED_DEPLOY_NAMES_SOL,
`
// SPDX-License-Identifier: MIT
pragma solidity >=0.8.13 <0.9;
/// ------------------------------------------------------------------------------------------------------------
///
/// NOTE: this file is auto-generated by ${path.basename(__filename)}, please DO NOT modify it directly.
/// NOTE: this file is auto-generated by ${path.basename(
__filename
)}, please DO NOT modify it directly.
///
/// ------------------------------------------------------------------------------------------------------------
import "src/diamonds/nayms/INayms.sol";
import { Nayms } from "src/diamonds/nayms/Nayms.sol";
import { InitDiamond } from "src/diamonds/nayms/InitDiamond.sol";
${facetNames.map((n, i) => `import { ${n}Facet } from "src/diamonds/nayms/facets/${n}Facet.sol";`).join("\n")}
${facetNames
.map(
(n, i) =>
`import { ${n}Facet } from "src/diamonds/nayms/facets/${n}Facet.sol";`
)
.join("\n")}
enum NaymsFacetAddressIndex {
${facetNames.join(",\n ")}
Expand All @@ -77,39 +118,58 @@ library LibGeneratedNaymsFacetHelpers {
// yul too slow, so fix stack too deep here
{
${facetNames.map((n, i) => {
const { interfaceName, methods } = facetData[n];
const selectors = methods.map((m, mi) => `f${i}[${mi}] = ${interfaceName}.${m}.selector;`)
${facetNames
.map((n, i) => {
const { interfaceName, methods } = facetData[n];
const selectors = methods.map(
(m, mi) => `f${i}[${mi}] = ${interfaceName}.${m}.selector;`
);
const str1 = `bytes4[] memory f${i} = new bytes4[](${methods.length});
${selectors.join("\n ")}`
const str1 = `bytes4[] memory f${i} = new bytes4[](${methods.length});
${selectors.join("\n ")}`;
const str2 = `cut[${i}] = IDiamondCut.FacetCut({
const str2 = `cut[${i}] = IDiamondCut.FacetCut({
facetAddress: address(facetAddresses[uint256(NaymsFacetAddressIndex.${n})]),
action: IDiamondCut.FacetCutAction.Add,
functionSelectors: f${i}
});`
return `${str1}\n ${str2}`
}).join("\n ")}
});`;
return `${str1}\n ${str2}`;
})
.join("\n ")}
}
}
function deployNaymsFacets() internal returns (address[] memory facetAddresses) {
facetAddresses = new address[](${facetNames.length});
${facetNames.map(n => `facetAddresses[uint256(NaymsFacetAddressIndex.${n})] = address(new ${n}Facet());`).join("\n ")}
${facetNames
.map(
(n) =>
`facetAddresses[uint256(NaymsFacetAddressIndex.${n})] = address(new ${n}Facet());`
)
.join("\n ")}
}
function deployNaymsFacets(NaymsFacetAddressIndex index) internal returns (address facetAddress) {
${facetNames.map(n => `if (index == NaymsFacetAddressIndex.${n}) { return address(new ${n}Facet()); }`).join("\n ")}
${facetNames
.map(
(n) =>
`if (index == NaymsFacetAddressIndex.${n}) { return address(new ${n}Facet()); }`
)
.join("\n ")}
}
function deployNaymsFacetsByName(string memory name) internal returns (address facetAddress) {
if (keccak256(abi.encodePacked(name)) == keccak256(abi.encodePacked("Nayms"))) { return address(new Nayms(msg.sender)); }
if (keccak256(abi.encodePacked(name)) == keccak256(abi.encodePacked("InitDiamond"))) { return address(new InitDiamond()); }
${facetNames.map(n => `if (keccak256(abi.encodePacked(name)) == keccak256(abi.encodePacked("${n}"))) { return address(new ${n}Facet()); }`).join("\n ")}
${facetNames
.map(
(n) =>
`if (keccak256(abi.encodePacked(name)) == keccak256(abi.encodePacked("${n}"))) { return address(new ${n}Facet()); }`
)
.join("\n ")}
}
}
`, { encoding: 'utf-8'})

`,
{ encoding: "utf-8" }
);
11 changes: 9 additions & 2 deletions foundry.toml
Expand Up @@ -15,8 +15,6 @@ optimizer = true
optimizer_runs = 200
via_ir = false
ignored_error_codes = []
fuzz_runs = 256
# fuzz_max_global_rejects = 9999999
ffi = false
# sender = '0xfcE918c07BD4c900941500A6632deB24bA7897Ce'
# tx_origin = '0xfcE918c07BD4c900941500A6632deB24bA7897Ce'
Expand Down Expand Up @@ -66,9 +64,18 @@ fs_permissions = [
{ access = "read", path = "./nayms_mnemonic.txt" },
]

[fuzz]
runs = 256
max_test_rejects = 65536
seed = '0x3e8'
dictionary_weight = 40
include_storage = true
include_push_bytes = true

[rpc_endpoints]
mainnet = "${ETH_MAINNET_RPC_URL}"
goerli = "${ETH_GOERLI_RPC_URL}"
sepolia = "${ETH_SEPOLIA_RPC_URL}"
anvil = "${LOCAL_RPC_URL}"
# See more config options https://github.com/foundry-rs/foundry/tree/master/config
# 0x00a329c0648769a73afac7f9381e08fb43dbea72
2 changes: 1 addition & 1 deletion lib/ds-test
2 changes: 1 addition & 1 deletion lib/forge-std
2 changes: 1 addition & 1 deletion package.json
@@ -1,6 +1,6 @@
{
"name": "@nayms/contracts",
"version": "3.4.6-p1",
"version": "3.4.6-qs2",
"main": "index.js",
"repository": "https://github.com/nayms/contracts-v3.git",
"author": "Kevin Park <kevin@fruitful.fi>",
Expand Down

0 comments on commit eec4700

Please sign in to comment.