add storage diff script and test it#131
Conversation
Summary of Test Results if Merged To Main:
🔒 Security AnalysisHigh Severity Issuesarbitrary-send-ethImpact: AtomWallet._call(address,uint256,bytes) (src/AtomWallet.sol#214-221) sends eth to arbitrary user Dangerous calls: - (success,result) = target.call{value: value}(data) (src/AtomWallet.sol#215) Affected Files:
View Detailed Findings
reentrancy-ethImpact: Reentrancy in EthMultiVault.batchDeposit(address,uint256[],uint256[]) (src/EthMultiVault.sol#1219-1252): External calls: - _transferFeesToProtocolMultisig(protocolFee) (src/EthMultiVault.sol#1248) - (success,None) = address(generalConfig.protocolMultisig).call{value: value}() (src/EthMultiVault.sol#1418) State variables written after the call(s): - shares[i] = _deposit(receiver,termIds[i],userDepositAfterprotocolFee) (src/EthMultiVault.sol#1247) - vaults[id].totalAssets = totalAssets (src/EthMultiVault.sol#1759) - vaults[id].balanceOf[to] += amount (src/EthMultiVault.sol#1714) - vaults[id].totalShares = totalShares (src/EthMultiVault.sol#1760) EthMultiVault.vaults (src/EthMultiVault.sol#97) can be used in cross function reentrancies: - EthMultiVault.convertToAssets(uint256,uint256) (src/EthMultiVault.sol#2166-2170) - EthMultiVault.convertToShares(uint256,uint256) (src/EthMultiVault.sol#2131-2135) - EthMultiVault.currentSharePrice(uint256) (src/EthMultiVault.sol#2067-2073) - EthMultiVault.getDepositSharesAndFees(uint256,uint256) (src/EthMultiVault.sol#1859-1886) - EthMultiVault.getRedeemAssetsAndFees(uint256,uint256) (src/EthMultiVault.sol#1926-1960) - EthMultiVault.getVaultStateForUser(uint256,address) (src/EthMultiVault.sol#2303-2307) - EthMultiVault.maxRedeem(address,uint256) (src/EthMultiVault.sol#2105-2108) - EthMultiVault.vaults (src/EthMultiVault.sol#97) Affected Files:
View Detailed Findings
Medium Severity IssuesView Medium Severity Issues##### incorrect-equality **Impact**: EthMultiVault._validateTimelock(bytes32) (src/EthMultiVault.sol#2400-2412) uses a dangerous strict equality: - timelock.readyTime == 0 (src/EthMultiVault.sol#2403)Affected Files:
Recommended Actions
⛽ Gas Analysis |
Summary of Test Results if Merged To Main:
🔒 Security AnalysisHigh Severity Issuesarbitrary-send-ethImpact: AtomWallet._call(address,uint256,bytes) (src/AtomWallet.sol#214-221) sends eth to arbitrary user Dangerous calls: - (success,result) = target.call{value: value}(data) (src/AtomWallet.sol#215) Affected Files:
View Detailed Findings
reentrancy-ethImpact: Reentrancy in EthMultiVault.batchDeposit(address,uint256[],uint256[]) (src/EthMultiVault.sol#1219-1252): External calls: - _transferFeesToProtocolMultisig(protocolFee) (src/EthMultiVault.sol#1248) - (success,None) = address(generalConfig.protocolMultisig).call{value: value}() (src/EthMultiVault.sol#1418) State variables written after the call(s): - shares[i] = _deposit(receiver,termIds[i],userDepositAfterprotocolFee) (src/EthMultiVault.sol#1247) - vaults[id].totalAssets = totalAssets (src/EthMultiVault.sol#1759) - vaults[id].balanceOf[to] += amount (src/EthMultiVault.sol#1714) - vaults[id].totalShares = totalShares (src/EthMultiVault.sol#1760) EthMultiVault.vaults (src/EthMultiVault.sol#97) can be used in cross function reentrancies: - EthMultiVault.convertToAssets(uint256,uint256) (src/EthMultiVault.sol#2166-2170) - EthMultiVault.convertToShares(uint256,uint256) (src/EthMultiVault.sol#2131-2135) - EthMultiVault.currentSharePrice(uint256) (src/EthMultiVault.sol#2067-2073) - EthMultiVault.getDepositSharesAndFees(uint256,uint256) (src/EthMultiVault.sol#1859-1886) - EthMultiVault.getRedeemAssetsAndFees(uint256,uint256) (src/EthMultiVault.sol#1926-1960) - EthMultiVault.getVaultStateForUser(uint256,address) (src/EthMultiVault.sol#2303-2307) - EthMultiVault.maxRedeem(address,uint256) (src/EthMultiVault.sol#2105-2108) - EthMultiVault.vaults (src/EthMultiVault.sol#97) Affected Files:
View Detailed Findings
Medium Severity IssuesView Medium Severity Issues##### incorrect-equality **Impact**: EthMultiVault._validateTimelock(bytes32) (src/EthMultiVault.sol#2400-2412) uses a dangerous strict equality: - timelock.readyTime == 0 (src/EthMultiVault.sol#2403)Affected Files:
Recommended Actions
⛽ Gas Analysis |
auroter
left a comment
auroter
left a comment
There was a problem hiding this comment.
Super cool, but let's omit the output of the script from the repo by adding it to gitignore, and add this to the github workflow instead to include it in the generated docs when we merge to main.
Summary of Test Results if Merged To Main:
🔒 Security AnalysisHigh Severity Issuesarbitrary-send-ethImpact: AtomWallet._call(address,uint256,bytes) (src/AtomWallet.sol#214-221) sends eth to arbitrary user Dangerous calls: - (success,result) = target.call{value: value}(data) (src/AtomWallet.sol#215) Affected Files:
View Detailed Findings
reentrancy-ethImpact: Reentrancy in EthMultiVault.batchDeposit(address,uint256[],uint256[]) (src/EthMultiVault.sol#1244-1277): External calls: - _transferFeesToProtocolMultisig(protocolFee) (src/EthMultiVault.sol#1273) - (success,None) = address(generalConfig.protocolMultisig).call{value: value}() (src/EthMultiVault.sol#1443) State variables written after the call(s): - shares[i] = _deposit(receiver,termIds[i],userDepositAfterprotocolFee) (src/EthMultiVault.sol#1272) - vaults[id].totalAssets = totalAssets (src/EthMultiVault.sol#1784) - vaults[id].balanceOf[to] += amount (src/EthMultiVault.sol#1739) - vaults[id].totalShares = totalShares (src/EthMultiVault.sol#1785) EthMultiVault.vaults (src/EthMultiVault.sol#97) can be used in cross function reentrancies: - EthMultiVault.convertToAssets(uint256,uint256) (src/EthMultiVault.sol#2191-2195) - EthMultiVault.convertToShares(uint256,uint256) (src/EthMultiVault.sol#2156-2160) - EthMultiVault.currentSharePrice(uint256) (src/EthMultiVault.sol#2092-2098) - EthMultiVault.getDepositSharesAndFees(uint256,uint256) (src/EthMultiVault.sol#1884-1911) - EthMultiVault.getRedeemAssetsAndFees(uint256,uint256) (src/EthMultiVault.sol#1951-1985) - EthMultiVault.getVaultStateForUser(uint256,address) (src/EthMultiVault.sol#2328-2332) - EthMultiVault.maxRedeem(address,uint256) (src/EthMultiVault.sol#2130-2133) - EthMultiVault.vaults (src/EthMultiVault.sol#97) Affected Files:
View Detailed Findings
Medium Severity IssuesView Medium Severity Issues##### incorrect-equality **Impact**: EthMultiVault._validateTimelock(bytes32) (src/EthMultiVault.sol#2425-2437) uses a dangerous strict equality: - timelock.readyTime == 0 (src/EthMultiVault.sol#2428)Affected Files:
Recommended Actions
⛽ Gas Analysis |
2 participants
In this PR, I added the
storage-diff.shscript to help us identify any possible errors in a storage layout for the new implementation contracts in an automated way. Also added a mock contract to test out the script, as well as the generalized script for deploying new implementation contracts as needed.The
storage-diff.shscript I added is inspired by and based on the similar script used by EigenLayer.