diff --git a/reports/adhoc-sol-files-highs-only-report.json b/reports/adhoc-sol-files-highs-only-report.json index 7e7bedea1..6cf4758ea 100644 --- a/reports/adhoc-sol-files-highs-only-report.json +++ b/reports/adhoc-sol-files-highs-only-report.json @@ -195,10 +195,7 @@ "weak-randomness", "pre-declared-local-variable-usage", "delete-nested-mapping", -<<<<<<< HEAD - "msg-value-in-loop" -======= + "msg-value-in-loop", "contract-locks-ether" ->>>>>>> dev ] } \ No newline at end of file diff --git a/reports/report.json b/reports/report.json index 4cd7ee856..00cc4b23c 100644 --- a/reports/report.json +++ b/reports/report.json @@ -1,12 +1,7 @@ { "files_summary": { -<<<<<<< HEAD - "total_source_units": 74, - "total_sloc": 2042 -======= - "total_source_units": 75, - "total_sloc": 2128 ->>>>>>> dev + "total_source_units": 76, + "total_sloc": 2183 }, "files_details": { "files_details": [ @@ -132,7 +127,7 @@ }, { "file_path": "src/MsgValueInLoop.sol", - "n_sloc": 46 + "n_sloc": 55 }, { "file_path": "src/MultipleConstructorSchemes.sol", @@ -317,11 +312,7 @@ ] }, "issue_count": { -<<<<<<< HEAD - "high": 33, -======= - "high": 34, ->>>>>>> dev + "high": 35, "low": 25 }, "high_issues": { @@ -1860,35 +1851,37 @@ ] }, { -<<<<<<< HEAD "title": "Loop contains `msg.value`.", - "description": "Do not use `msg.value` in loops since it may be misleading when calculating the total amount.It's recommended to prepare additional variables outside the loop to do the required calculation.", + "description": "Provide an explicit array of amounts alongside the receivers array, and check that the sum of all amounts matches `msg.value`.", "detector_name": "msg-value-in-loop", "instances": [ { "contract_path": "src/MsgValueInLoop.sol", - "line_no": 10, - "src": "249:107", - "src_char": "249:107" + "line_no": 12, + "src": "287:107", + "src_char": "287:107" }, { "contract_path": "src/MsgValueInLoop.sol", - "line_no": 21, - "src": "530:94", - "src_char": "530:94" + "line_no": 38, + "src": "982:94", + "src_char": "982:94" }, { "contract_path": "src/MsgValueInLoop.sol", - "line_no": 37, - "src": "955:93", - "src_char": "955:93" + "line_no": 54, + "src": "1407:93", + "src_char": "1407:93" }, { "contract_path": "src/MsgValueInLoop.sol", - "line_no": 54, - "src": "1382:97", - "src_char": "1382:97" -======= + "line_no": 71, + "src": "1834:97", + "src_char": "1834:97" + } + ] + }, + { "title": "Contract locks Ether without a withdraw function.", "description": "It appears that the contract includes a payable function to accept Ether but lacks a corresponding function to withdraw it, which leads to the Ether being locked in the contract. To resolve this issue, please implement a public or external function that allows for the withdrawal of Ether from the contract.", "detector_name": "contract-locks-ether", @@ -1917,12 +1910,41 @@ "src": "630:11", "src_char": "630:11" }, + { + "contract_path": "src/MsgValueInLoop.sol", + "line_no": 7, + "src": "103:15", + "src_char": "103:15" + }, + { + "contract_path": "src/MsgValueInLoop.sol", + "line_no": 19, + "src": "421:19", + "src_char": "421:19" + }, + { + "contract_path": "src/MsgValueInLoop.sol", + "line_no": 33, + "src": "827:15", + "src_char": "827:15" + }, + { + "contract_path": "src/MsgValueInLoop.sol", + "line_no": 48, + "src": "1227:15", + "src_char": "1227:15" + }, + { + "contract_path": "src/MsgValueInLoop.sol", + "line_no": 65, + "src": "1651:15", + "src_char": "1651:15" + }, { "contract_path": "src/eth2/DepositContract.sol", "line_no": 58, "src": "4547:15", "src_char": "3059:15" ->>>>>>> dev } ] } @@ -2388,27 +2410,33 @@ }, { "contract_path": "src/MsgValueInLoop.sol", - "line_no": 8, - "src": "168:3", - "src_char": "168:3" + "line_no": 10, + "src": "177:3", + "src_char": "177:3" }, { "contract_path": "src/MsgValueInLoop.sol", - "line_no": 19, - "src": "449:3", - "src_char": "449:3" + "line_no": 22, + "src": "499:4", + "src_char": "499:4" }, { "contract_path": "src/MsgValueInLoop.sol", - "line_no": 34, - "src": "849:3", - "src_char": "849:3" + "line_no": 36, + "src": "901:3", + "src_char": "901:3" }, { "contract_path": "src/MsgValueInLoop.sol", "line_no": 51, - "src": "1273:3", - "src_char": "1273:3" + "src": "1301:3", + "src_char": "1301:3" + }, + { + "contract_path": "src/MsgValueInLoop.sol", + "line_no": 68, + "src": "1725:3", + "src_char": "1725:3" }, { "contract_path": "src/StateVariables.sol", @@ -3640,21 +3668,21 @@ }, { "contract_path": "src/MsgValueInLoop.sol", - "line_no": 26, - "src": "645:8", - "src_char": "645:8" + "line_no": 43, + "src": "1097:8", + "src_char": "1097:8" }, { "contract_path": "src/MsgValueInLoop.sol", - "line_no": 43, - "src": "1069:8", - "src_char": "1069:8" + "line_no": 60, + "src": "1521:8", + "src_char": "1521:8" }, { "contract_path": "src/MsgValueInLoop.sol", - "line_no": 60, - "src": "1500:8", - "src_char": "1500:8" + "line_no": 77, + "src": "1952:8", + "src_char": "1952:8" }, { "contract_path": "src/SendEtherNoChecks.sol", @@ -4062,10 +4090,7 @@ "weak-randomness", "pre-declared-local-variable-usage", "delete-nested-mapping", -<<<<<<< HEAD - "msg-value-in-loop" -======= + "msg-value-in-loop", "contract-locks-ether" ->>>>>>> dev ] } \ No newline at end of file diff --git a/reports/report.md b/reports/report.md index e269a7748..e4e4ca2c4 100644 --- a/reports/report.md +++ b/reports/report.md @@ -35,22 +35,14 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati - [H-25: RTLO character detected in file. \u{202e}](#h-25-rtlo-character-detected-in-file-u202e) - [H-26: Return value of the function call is not checked.](#h-26-return-value-of-the-function-call-is-not-checked) - [H-27: Dangerous unary operator found in assignment.](#h-27-dangerous-unary-operator-found-in-assignment) -<<<<<<< HEAD - - [H-28: Dangerous strict equality checks on contract balances.](#h-28-dangerous-strict-equality-checks-on-contract-balances) - - [H-29: Compiler Bug: Signed array in storage detected for compiler version `<0.5.10`](#h-29-compiler-bug-signed-array-in-storage-detected-for-compiler-version-0510) - - [H-30: Weak Randomness](#h-30-weak-randomness) - - [H-31: Usage of variable before declaration.](#h-31-usage-of-variable-before-declaration) - - [H-32: Deletion from a nested mappping.](#h-32-deletion-from-a-nested-mappping) - - [H-33: Loop contains `msg.value`.](#h-33-loop-contains-msgvalue) -======= - [H-28: Tautology or Contradiction in comparison.](#h-28-tautology-or-contradiction-in-comparison) - [H-29: Dangerous strict equality checks on contract balances.](#h-29-dangerous-strict-equality-checks-on-contract-balances) - [H-30: Compiler Bug: Signed array in storage detected for compiler version `<0.5.10`](#h-30-compiler-bug-signed-array-in-storage-detected-for-compiler-version-0510) - [H-31: Weak Randomness](#h-31-weak-randomness) - [H-32: Usage of variable before declaration.](#h-32-usage-of-variable-before-declaration) - [H-33: Deletion from a nested mappping.](#h-33-deletion-from-a-nested-mappping) - - [H-34: Contract locks Ether without a withdraw function.](#h-34-contract-locks-ether-without-a-withdraw-function) ->>>>>>> dev + - [H-34: Loop contains `msg.value`.](#h-34-loop-contains-msgvalue) + - [H-35: Contract locks Ether without a withdraw function.](#h-35-contract-locks-ether-without-a-withdraw-function) - [Low Issues](#low-issues) - [L-1: Centralization Risk for trusted owners](#l-1-centralization-risk-for-trusted-owners) - [L-2: Solmate's SafeTransferLib does not check for token contract's existence](#l-2-solmates-safetransferlib-does-not-check-for-token-contracts-existence) @@ -85,13 +77,8 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Key | Value | | --- | --- | -<<<<<<< HEAD -| .sol Files | 74 | -| Total nSLOC | 2042 | -======= -| .sol Files | 75 | -| Total nSLOC | 2128 | ->>>>>>> dev +| .sol Files | 76 | +| Total nSLOC | 2183 | ## Files Details @@ -128,7 +115,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | src/InternalFunctions.sol | 22 | | src/KeccakContract.sol | 21 | | src/MisusedBoolean.sol | 67 | -| src/MsgValueInLoop.sol | 46 | +| src/MsgValueInLoop.sol | 55 | | src/MultipleConstructorSchemes.sol | 10 | | src/OnceModifierExample.sol | 8 | | src/PreDeclaredVarUsage.sol | 9 | @@ -174,22 +161,14 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | src/reused_contract_name/ContractB.sol | 7 | | src/uniswap/UniswapV2Swapper.sol | 50 | | src/uniswap/UniswapV3Swapper.sol | 150 | -<<<<<<< HEAD -| **Total** | **2042** | -======= -| **Total** | **2128** | ->>>>>>> dev +| **Total** | **2183** | ## Issue Summary | Category | No. of Issues | | --- | --- | -<<<<<<< HEAD -| High | 33 | -======= -| High | 34 | ->>>>>>> dev +| High | 35 | | Low | 25 | @@ -1862,42 +1841,46 @@ A deletion in a structure containing a mapping will not delete the mapping. The -<<<<<<< HEAD -## H-33: Loop contains `msg.value`. +## H-34: Loop contains `msg.value`. -Do not use `msg.value` in loops since it may be misleading when calculating the total amount.It's recommended to prepare additional variables outside the loop to do the required calculation. +Provide an explicit array of amounts alongside the receivers array, and check that the sum of all amounts matches `msg.value`.
4 Found Instances -- Found in src/MsgValueInLoop.sol [Line: 10](../tests/contract-playground/src/MsgValueInLoop.sol#L10) +- Found in src/MsgValueInLoop.sol [Line: 12](../tests/contract-playground/src/MsgValueInLoop.sol#L12) ```solidity for (uint256 i = 0; i < receivers.length; i++) { ``` -- Found in src/MsgValueInLoop.sol [Line: 21](../tests/contract-playground/src/MsgValueInLoop.sol#L21) +- Found in src/MsgValueInLoop.sol [Line: 38](../tests/contract-playground/src/MsgValueInLoop.sol#L38) ```solidity for (uint256 i = 0; i < receivers.length; i++) { ``` -- Found in src/MsgValueInLoop.sol [Line: 37](../tests/contract-playground/src/MsgValueInLoop.sol#L37) +- Found in src/MsgValueInLoop.sol [Line: 54](../tests/contract-playground/src/MsgValueInLoop.sol#L54) ```solidity while (i < receivers.length) { ``` -- Found in src/MsgValueInLoop.sol [Line: 54](../tests/contract-playground/src/MsgValueInLoop.sol#L54) +- Found in src/MsgValueInLoop.sol [Line: 71](../tests/contract-playground/src/MsgValueInLoop.sol#L71) ```solidity do { -======= -## H-34: Contract locks Ether without a withdraw function. + ``` + +
+ + + +## H-35: Contract locks Ether without a withdraw function. It appears that the contract includes a payable function to accept Ether but lacks a corresponding function to withdraw it, which leads to the Ether being locked in the contract. To resolve this issue, please implement a public or external function that allows for the withdrawal of Ether from the contract. -
5 Found Instances +
10 Found Instances - Found in src/ContractLocksEther.sol [Line: 5](../tests/contract-playground/src/ContractLocksEther.sol#L5) @@ -1924,11 +1907,40 @@ It appears that the contract includes a payable function to accept Ether but lac contract EmptyBlocks { ``` +- Found in src/MsgValueInLoop.sol [Line: 7](../tests/contract-playground/src/MsgValueInLoop.sol#L7) + + ```solidity + contract MsgValueInLoop1 { + ``` + +- Found in src/MsgValueInLoop.sol [Line: 19](../tests/contract-playground/src/MsgValueInLoop.sol#L19) + + ```solidity + contract MsgValueOutsideLoop { + ``` + +- Found in src/MsgValueInLoop.sol [Line: 33](../tests/contract-playground/src/MsgValueInLoop.sol#L33) + + ```solidity + contract MsgValueInLoop2 { + ``` + +- Found in src/MsgValueInLoop.sol [Line: 48](../tests/contract-playground/src/MsgValueInLoop.sol#L48) + + ```solidity + contract MsgValueInLoop3 { + ``` + +- Found in src/MsgValueInLoop.sol [Line: 65](../tests/contract-playground/src/MsgValueInLoop.sol#L65) + + ```solidity + contract MsgValueInLoop4 { + ``` + - Found in src/eth2/DepositContract.sol [Line: 58](../tests/contract-playground/src/eth2/DepositContract.sol#L58) ```solidity contract DepositContract is IDepositContract, ERC165 { ->>>>>>> dev ```
@@ -2173,11 +2185,7 @@ ERC20 functions may not behave as expected. For example: return values are not a Consider using a specific version of Solidity in your contracts instead of a wide version. For example, instead of `pragma solidity ^0.8.0;`, use `pragma solidity 0.8.0;` -<<<<<<< HEAD -
20 Found Instances -======= -
21 Found Instances ->>>>>>> dev +
22 Found Instances - Found in src/CompilerBugStorageSignedIntegerArray.sol [Line: 2](../tests/contract-playground/src/CompilerBugStorageSignedIntegerArray.sol#L2) @@ -2367,11 +2375,7 @@ Check for `address(0)` when assigning values to address state variables. Instead of marking a function as `public`, consider marking it as `external` if it is not used internally. -<<<<<<< HEAD -
27 Found Instances -======= -
29 Found Instances ->>>>>>> dev +
34 Found Instances - Found in src/ArbitraryTransferFrom.sol [Line: 28](../tests/contract-playground/src/ArbitraryTransferFrom.sol#L28) @@ -2434,19 +2438,19 @@ Instead of marking a function as `public`, consider marking it as `external` if function setNumber(uint256 newNumber) public { ``` -- Found in src/MsgValueInLoop.sol [Line: 8](../tests/contract-playground/src/MsgValueInLoop.sol#L8) +- Found in src/MsgValueInLoop.sol [Line: 10](../tests/contract-playground/src/MsgValueInLoop.sol#L10) ```solidity function bad(address[] memory receivers) public payable { ``` -- Found in src/MsgValueInLoop.sol [Line: 19](../tests/contract-playground/src/MsgValueInLoop.sol#L19) +- Found in src/MsgValueInLoop.sol [Line: 22](../tests/contract-playground/src/MsgValueInLoop.sol#L22) ```solidity - function bad(address[] memory receivers) public payable { + function good(address[] memory receivers) public payable { ``` -- Found in src/MsgValueInLoop.sol [Line: 34](../tests/contract-playground/src/MsgValueInLoop.sol#L34) +- Found in src/MsgValueInLoop.sol [Line: 36](../tests/contract-playground/src/MsgValueInLoop.sol#L36) ```solidity function bad(address[] memory receivers) public payable { @@ -2458,6 +2462,12 @@ Instead of marking a function as `public`, consider marking it as `external` if function bad(address[] memory receivers) public payable { ``` +- Found in src/MsgValueInLoop.sol [Line: 68](../tests/contract-playground/src/MsgValueInLoop.sol#L68) + + ```solidity + function bad(address[] memory receivers) public payable { + ``` + - Found in src/StateVariables.sol [Line: 47](../tests/contract-playground/src/StateVariables.sol#L47) ```solidity @@ -3091,7 +3101,7 @@ Using `ERC721::_mint()` can mint ERC721 tokens to addresses which don't support Solc compiler version 0.8.20 switches the default target EVM version to Shanghai, which means that the generated bytecode will include PUSH0 opcodes. Be sure to select the appropriate EVM version in case you intend to deploy on a chain other than mainnet like L2 chains that may not support PUSH0, otherwise deployment of your contracts will fail. -
29 Found Instances +
30 Found Instances - Found in src/AdminContract.sol [Line: 2](../tests/contract-playground/src/AdminContract.sol#L2) @@ -3714,19 +3724,19 @@ Instead of separating the logic into a separate function, consider inlining the function internalSet2(uint256 _newValue) internal { ``` -- Found in src/MsgValueInLoop.sol [Line: 26](../tests/contract-playground/src/MsgValueInLoop.sol#L26) +- Found in src/MsgValueInLoop.sol [Line: 43](../tests/contract-playground/src/MsgValueInLoop.sol#L43) ```solidity function addToBal(address[] memory receivers, uint256 index) internal { ``` -- Found in src/MsgValueInLoop.sol [Line: 43](../tests/contract-playground/src/MsgValueInLoop.sol#L43) +- Found in src/MsgValueInLoop.sol [Line: 60](../tests/contract-playground/src/MsgValueInLoop.sol#L60) ```solidity function addToBal(address[] memory receivers, uint256 index) internal { ``` -- Found in src/MsgValueInLoop.sol [Line: 60](../tests/contract-playground/src/MsgValueInLoop.sol#L60) +- Found in src/MsgValueInLoop.sol [Line: 77](../tests/contract-playground/src/MsgValueInLoop.sol#L77) ```solidity function addToBal(address[] memory receivers, uint256 index) internal { diff --git a/reports/report.sarif b/reports/report.sarif index 61e244f38..07535f1fe 100644 --- a/reports/report.sarif +++ b/reports/report.sarif @@ -2694,70 +2694,92 @@ { "physicalLocation": { "artifactLocation": { -<<<<<<< HEAD "uri": "src/MsgValueInLoop.sol" }, "region": { "byteLength": 107, - "byteOffset": 249 -======= - "uri": "src/ContractLocksEther.sol" + "byteOffset": 287 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/MsgValueInLoop.sol" }, "region": { - "byteLength": 10, - "byteOffset": 73 ->>>>>>> dev + "byteLength": 94, + "byteOffset": 982 } } }, { "physicalLocation": { "artifactLocation": { -<<<<<<< HEAD "uri": "src/MsgValueInLoop.sol" }, "region": { - "byteLength": 94, - "byteOffset": 530 -======= + "byteLength": 93, + "byteOffset": 1407 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/MsgValueInLoop.sol" + }, + "region": { + "byteLength": 97, + "byteOffset": 1834 + } + } + } + ], + "message": { + "text": "Provide an explicit array of amounts alongside the receivers array, and check that the sum of all amounts matches `msg.value`." + }, + "ruleId": "msg-value-in-loop" + }, + { + "level": "warning", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { "uri": "src/ContractLocksEther.sol" }, "region": { - "byteLength": 11, - "byteOffset": 822 ->>>>>>> dev + "byteLength": 10, + "byteOffset": 73 } } }, { "physicalLocation": { "artifactLocation": { -<<<<<<< HEAD - "uri": "src/MsgValueInLoop.sol" + "uri": "src/ContractLocksEther.sol" }, "region": { - "byteLength": 93, - "byteOffset": 955 -======= + "byteLength": 11, + "byteOffset": 822 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { "uri": "src/EmptyBlocks.sol" }, "region": { "byteLength": 39, "byteOffset": 344 ->>>>>>> dev } } }, { "physicalLocation": { "artifactLocation": { -<<<<<<< HEAD - "uri": "src/MsgValueInLoop.sol" - }, - "region": { - "byteLength": 97, - "byteOffset": 1382 -======= "uri": "src/EmptyBlocks.sol" }, "region": { @@ -2766,6 +2788,61 @@ } } }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/MsgValueInLoop.sol" + }, + "region": { + "byteLength": 15, + "byteOffset": 103 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/MsgValueInLoop.sol" + }, + "region": { + "byteLength": 19, + "byteOffset": 421 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/MsgValueInLoop.sol" + }, + "region": { + "byteLength": 15, + "byteOffset": 827 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/MsgValueInLoop.sol" + }, + "region": { + "byteLength": 15, + "byteOffset": 1227 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/MsgValueInLoop.sol" + }, + "region": { + "byteLength": 15, + "byteOffset": 1651 + } + } + }, { "physicalLocation": { "artifactLocation": { @@ -2774,21 +2851,14 @@ "region": { "byteLength": 15, "byteOffset": 4547 ->>>>>>> dev } } } ], "message": { -<<<<<<< HEAD - "text": "Do not use `msg.value` in loops since it may be misleading when calculating the total amount.It's recommended to prepare additional variables outside the loop to do the required calculation." - }, - "ruleId": "msg-value-in-loop" -======= "text": "It appears that the contract includes a payable function to accept Ether but lacks a corresponding function to withdraw it, which leads to the Ether being locked in the contract. To resolve this issue, please implement a public or external function that allows for the withdrawal of Ether from the contract." }, "ruleId": "contract-locks-ether" ->>>>>>> dev }, { "level": "note", @@ -3600,7 +3670,18 @@ }, "region": { "byteLength": 3, - "byteOffset": 168 + "byteOffset": 177 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/MsgValueInLoop.sol" + }, + "region": { + "byteLength": 4, + "byteOffset": 499 } } }, @@ -3611,7 +3692,7 @@ }, "region": { "byteLength": 3, - "byteOffset": 449 + "byteOffset": 901 } } }, @@ -3622,7 +3703,7 @@ }, "region": { "byteLength": 3, - "byteOffset": 849 + "byteOffset": 1301 } } }, @@ -3633,7 +3714,7 @@ }, "region": { "byteLength": 3, - "byteOffset": 1273 + "byteOffset": 1725 } } }, @@ -5857,7 +5938,7 @@ }, "region": { "byteLength": 8, - "byteOffset": 645 + "byteOffset": 1097 } } }, @@ -5868,7 +5949,7 @@ }, "region": { "byteLength": 8, - "byteOffset": 1069 + "byteOffset": 1521 } } }, @@ -5879,7 +5960,7 @@ }, "region": { "byteLength": 8, - "byteOffset": 1500 + "byteOffset": 1952 } } },