From a8117c8c97694f03902fcd1ac640b6fb8692047a Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Sun, 6 Oct 2024 20:50:46 +0530 Subject: [PATCH] state change hint --- .../high/state_change_after_ext_call.rs | 17 ++++++++++++-- reports/ccip-functions-report.md | 6 +++++ reports/report.json | 18 ++++++++++----- reports/report.md | 6 +++++ reports/report.sarif | 18 +++++++++++++++ reports/templegold-report.md | 23 +++++++++++++++++++ 6 files changed, 80 insertions(+), 8 deletions(-) diff --git a/aderyn_core/src/detect/high/state_change_after_ext_call.rs b/aderyn_core/src/detect/high/state_change_after_ext_call.rs index cb19a75f..5d45d216 100644 --- a/aderyn_core/src/detect/high/state_change_after_ext_call.rs +++ b/aderyn_core/src/detect/high/state_change_after_ext_call.rs @@ -3,7 +3,7 @@ use std::{ error::Error, }; -use crate::ast::NodeID; +use crate::{ast::NodeID, context::browser::Peek}; use crate::{ capture, @@ -58,7 +58,20 @@ impl IssueDetector for StateChangeAfterExternalCallDetector { if let Some(external_call_ast_node) = external_call_cfg_node.reflect(context) { - capture!(self, context, external_call_ast_node); + let state_change_cfg_node = + cfg.nodes.get(&state_change).expect("cfg is corrupted"); + + if let Some(state_change_ast_node) = + state_change_cfg_node.reflect(context) + { + if let Some(state_change_code) = state_change_ast_node.peek(context) + { + let hint = + format!("State is changed at: `{}`", state_change_code); + + capture!(self, context, external_call_ast_node, hint); + } + } } } } diff --git a/reports/ccip-functions-report.md b/reports/ccip-functions-report.md index 1266a31d..ebc4ec07 100644 --- a/reports/ccip-functions-report.md +++ b/reports/ccip-functions-report.md @@ -502,36 +502,42 @@ In most cases it is a best practice to perform the state change before making an - Found in src/v0.8/functions/dev/v1_X/FunctionsBilling.sol [Line: 403](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/FunctionsBilling.sol#L403) + State is changed at: `s_withdrawableTokens[transmitters[i]] = 0` ```solidity IFunctionsSubscriptions(address(_getRouter())).oracleWithdraw(transmitters[i], balance); ``` - Found in src/v0.8/functions/dev/v1_X/FunctionsSubscriptions.sol [Line: 519](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/FunctionsSubscriptions.sol#L519) + State is changed at: `delete s_requestCommitments[requestId]` ```solidity IFunctionsBilling(request.coordinator).deleteCommitment(requestId); ``` - Found in src/v0.8/functions/v1_0_0/FunctionsBilling.sol [Line: 342](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_0_0/FunctionsBilling.sol#L342) + State is changed at: `s_withdrawableTokens[transmitters[i]] = 0` ```solidity IFunctionsSubscriptions(address(_getRouter())).oracleWithdraw(transmitters[i], balance); ``` - Found in src/v0.8/functions/v1_0_0/FunctionsSubscriptions.sol [Line: 519](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_0_0/FunctionsSubscriptions.sol#L519) + State is changed at: `s_subscriptions[subscriptionId].blockedBalance -= request.estimatedTotalCostJuels` ```solidity IFunctionsBilling(request.coordinator).deleteCommitment(requestId); ``` - Found in src/v0.8/functions/v1_1_0/FunctionsBilling.sol [Line: 360](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_1_0/FunctionsBilling.sol#L360) + State is changed at: `s_withdrawableTokens[transmitters[i]] = 0` ```solidity IFunctionsSubscriptions(address(_getRouter())).oracleWithdraw(transmitters[i], balance); ``` - Found in src/v0.8/functions/v1_3_0/FunctionsBilling.sol [Line: 401](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_3_0/FunctionsBilling.sol#L401) + State is changed at: `s_withdrawableTokens[transmitters[i]] = 0` ```solidity IFunctionsSubscriptions(address(_getRouter())).oracleWithdraw(transmitters[i], balance); ``` diff --git a/reports/report.json b/reports/report.json index 7c1c97ac..6d44a036 100644 --- a/reports/report.json +++ b/reports/report.json @@ -2614,37 +2614,43 @@ "contract_path": "src/StateChangeAfterExternalCall.sol", "line_no": 24, "src": "588:15", - "src_char": "588:15" + "src_char": "588:15", + "hint": "State is changed at: `s_useMe += 1`" }, { "contract_path": "src/StateChangeAfterExternalCall.sol", "line_no": 33, "src": "735:15", - "src_char": "735:15" + "src_char": "735:15", + "hint": "State is changed at: `s_useMe -= 1`" }, { "contract_path": "src/StateChangeAfterExternalCall.sol", "line_no": 52, "src": "1272:15", - "src_char": "1272:15" + "src_char": "1272:15", + "hint": "State is changed at: `s_useMe += 4`" }, { "contract_path": "src/Trump.sol", "line_no": 342, "src": "11731:110", - "src_char": "11731:110" + "src_char": "11731:110", + "hint": "State is changed at: `tradingOpen = true`" }, { "contract_path": "src/Trump.sol", "line_no": 343, "src": "11851:129", - "src_char": "11851:129" + "src_char": "11851:129", + "hint": "State is changed at: `tradingOpen = true`" }, { "contract_path": "src/Trump.sol", "line_no": 344, "src": "11990:71", - "src_char": "11990:71" + "src_char": "11990:71", + "hint": "State is changed at: `firstBlock = block.number`" } ] } diff --git a/reports/report.md b/reports/report.md index 4e6791cc..eb3534db 100644 --- a/reports/report.md +++ b/reports/report.md @@ -2554,36 +2554,42 @@ In most cases it is a best practice to perform the state change before making an - Found in src/StateChangeAfterExternalCall.sol [Line: 24](../tests/contract-playground/src/StateChangeAfterExternalCall.sol#L24) + State is changed at: `s_useMe += 1` ```solidity s_actor.hello(); ``` - Found in src/StateChangeAfterExternalCall.sol [Line: 33](../tests/contract-playground/src/StateChangeAfterExternalCall.sol#L33) + State is changed at: `s_useMe -= 1` ```solidity s_actor.hello(); ``` - Found in src/StateChangeAfterExternalCall.sol [Line: 52](../tests/contract-playground/src/StateChangeAfterExternalCall.sol#L52) + State is changed at: `s_useMe += 4` ```solidity s_actor.hello(); ``` - Found in src/Trump.sol [Line: 342](../tests/contract-playground/src/Trump.sol#L342) + State is changed at: `swapEnabled = true` ```solidity uniswapV2Pair = IUniswapV2Factory(uniswapV2Router.factory()).createPair(address(this), uniswapV2Router.WETH()); ``` - Found in src/Trump.sol [Line: 343](../tests/contract-playground/src/Trump.sol#L343) + State is changed at: `firstBlock = block.number` ```solidity uniswapV2Router.addLiquidityETH{value: address(this).balance}(address(this),balanceOf(address(this)),0,0,owner(),block.timestamp); ``` - Found in src/Trump.sol [Line: 344](../tests/contract-playground/src/Trump.sol#L344) + State is changed at: `firstBlock = block.number` ```solidity IERC20(uniswapV2Pair).approve(address(uniswapV2Router), type(uint).max); ``` diff --git a/reports/report.sarif b/reports/report.sarif index e76fba55..1f22e487 100644 --- a/reports/report.sarif +++ b/reports/report.sarif @@ -3758,6 +3758,9 @@ "level": "warning", "locations": [ { + "message": { + "text": "State is changed at: `s_useMe += 1`" + }, "physicalLocation": { "artifactLocation": { "uri": "src/StateChangeAfterExternalCall.sol" @@ -3769,6 +3772,9 @@ } }, { + "message": { + "text": "State is changed at: `s_useMe -= 1`" + }, "physicalLocation": { "artifactLocation": { "uri": "src/StateChangeAfterExternalCall.sol" @@ -3780,6 +3786,9 @@ } }, { + "message": { + "text": "State is changed at: `s_useMe += 4`" + }, "physicalLocation": { "artifactLocation": { "uri": "src/StateChangeAfterExternalCall.sol" @@ -3791,6 +3800,9 @@ } }, { + "message": { + "text": "State is changed at: `swapEnabled = true`" + }, "physicalLocation": { "artifactLocation": { "uri": "src/Trump.sol" @@ -3802,6 +3814,9 @@ } }, { + "message": { + "text": "State is changed at: `firstBlock = block.number`" + }, "physicalLocation": { "artifactLocation": { "uri": "src/Trump.sol" @@ -3813,6 +3828,9 @@ } }, { + "message": { + "text": "State is changed at: `firstBlock = block.number`" + }, "physicalLocation": { "artifactLocation": { "uri": "src/Trump.sol" diff --git a/reports/templegold-report.md b/reports/templegold-report.md index 444a8510..54b46228 100644 --- a/reports/templegold-report.md +++ b/reports/templegold-report.md @@ -558,138 +558,161 @@ In most cases it is a best practice to perform the state change before making an - Found in contracts/amo/Ramos.sol [Line: 195](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L195) + State is changed at: `tokenVault = IRamosTokenVault(vault)` ```solidity protocolToken.approve(previousVault, 0); ``` - Found in contracts/amo/Ramos.sol [Line: 196](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L196) + State is changed at: `tokenVault = IRamosTokenVault(vault)` ```solidity quoteToken.approve(previousVault, 0); ``` - Found in contracts/core/MultiOtcOffer.sol [Line: 68](../tests/2024-07-templegold/protocol/contracts/core/MultiOtcOffer.sol#L68) + State is changed at: `marketInfo.scalar = 10 ** scaleDecimals` ```solidity uint256 scaleDecimals = marketInfo.offerPricingToken == OfferPricingToken.UserBuyToken ``` - Found in contracts/core/OpsManager.sol [Line: 36](../tests/2024-07-templegold/protocol/contracts/core/OpsManager.sol#L36) + State is changed at: `vaultedTemple = new VaultedTemple(_templeToken, address(templeExposure))` ```solidity templeExposure.setMinterState(address(this), true); ``` - Found in contracts/core/OtcOffer.sol [Line: 91](../tests/2024-07-templegold/protocol/contracts/core/OtcOffer.sol#L91) + State is changed at: `scalar = 10 ** scaleDecimals` ```solidity uint256 scaleDecimals = offerPricingToken == OfferPricingToken.UserBuyToken ``` - Found in contracts/fakes/templegold/TempleGoldStakingMock.sol [Line: 158](../tests/2024-07-templegold/protocol/contracts/fakes/templegold/TempleGoldStakingMock.sol#L158) + State is changed at: `_accountLastStakeIndex[msg.sender] = ++_lastIndex` ```solidity uint256 amount = previousStaking.migrateWithdraw(msg.sender, index); ``` - Found in contracts/fakes/v2/strategies/DsrBaseStrategyTestnet.sol [Line: 52](../tests/2024-07-templegold/protocol/contracts/fakes/v2/strategies/DsrBaseStrategyTestnet.sol#L52) + State is changed at: `daiSavingsRate = _newRate` ```solidity _checkpointDaiBalance(daiToken.balanceOf(address(this))); ``` - Found in contracts/governance/ElderElection.sol [Line: 74](../tests/2024-07-templegold/protocol/contracts/governance/ElderElection.sol#L74) + State is changed at: `numCandidates += 1` ```solidity templars.checkExists(discordId); ``` - Found in contracts/governance/ElderElection.sol [Line: 87](../tests/2024-07-templegold/protocol/contracts/governance/ElderElection.sol#L87) + State is changed at: `numCandidates -= 1` ```solidity templars.checkExists(discordId); ``` - Found in contracts/governance/TemplarMetadata.sol [Line: 32](../tests/2024-07-templegold/protocol/contracts/governance/TemplarMetadata.sol#L32) + State is changed at: `templeRole[discordId] = _templeRole` ```solidity templars.checkExists(discordId); ``` - Found in contracts/templegold/SpiceAuction.sol [Line: 159](../tests/2024-07-templegold/protocol/contracts/templegold/SpiceAuction.sol#L159) + State is changed at: `uint128 endTime = info.endTime = startTime + config.duration` ```solidity uint256 balance = IERC20(auctionToken).balanceOf(address(this)); ``` - Found in contracts/templegold/SpiceAuction.sol [Line: 194](../tests/2024-07-templegold/protocol/contracts/templegold/SpiceAuction.sol#L194) + State is changed at: `info.totalBidTokenAmount += amount` ```solidity uint256 _bidTokenAmountBefore = IERC20(bidToken).balanceOf(_recipient); ``` - Found in contracts/templegold/SpiceAuction.sol [Line: 196](../tests/2024-07-templegold/protocol/contracts/templegold/SpiceAuction.sol#L196) + State is changed at: `info.totalBidTokenAmount += amount` ```solidity uint256 _bidTokenAmountAfter = IERC20(bidToken).balanceOf(_recipient); ``` - Found in contracts/v2/TreasuryReservesVault.sol [Line: 148](../tests/2024-07-templegold/protocol/contracts/v2/TreasuryReservesVault.sol#L148) + State is changed at: `tpiOracle = _tpiOracle` ```solidity if (_tpiOracle.treasuryPriceIndex() == 0) revert CommonEventsAndErrors.InvalidParam(); ``` - Found in contracts/v2/TreasuryReservesVault.sol [Line: 296](../tests/2024-07-templegold/protocol/contracts/v2/TreasuryReservesVault.sol#L296) + State is changed at: `delete strategies[strategy]` ```solidity _outstandingDebt = borrowTokens[_token].dToken.burnAll(strategy); ``` - Found in contracts/v2/strategies/DsrBaseStrategy.sol [Line: 55](../tests/2024-07-templegold/protocol/contracts/v2/strategies/DsrBaseStrategy.sol#L55) + State is changed at: `pot = IMakerDaoPotLike(_pot)` ```solidity IMakerDaoVatLike vat = IMakerDaoVatLike(daiJoin.vat()); ``` - Found in contracts/v2/strategies/DsrBaseStrategy.sol [Line: 57](../tests/2024-07-templegold/protocol/contracts/v2/strategies/DsrBaseStrategy.sol#L57) + State is changed at: `daiToken = IERC20(_daiToken)` ```solidity vat.hope(address(daiJoin)); ``` - Found in contracts/v2/strategies/DsrBaseStrategy.sol [Line: 58](../tests/2024-07-templegold/protocol/contracts/v2/strategies/DsrBaseStrategy.sol#L58) + State is changed at: `daiToken = IERC20(_daiToken)` ```solidity vat.hope(address(pot)); ``` - Found in contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol [Line: 173](../tests/2024-07-templegold/protocol/contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol#L173) + State is changed at: `totalCollateral -= amount` ```solidity circuitBreakerProxy.preCheck( ``` - Found in contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol [Line: 213](../tests/2024-07-templegold/protocol/contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol#L213) + State is changed at: `debtTokenData.totalDebt = _cache.totalDebt = _cache.totalDebt + amount` ```solidity circuitBreakerProxy.preCheck( ``` - Found in contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol [Line: 351](../tests/2024-07-templegold/protocol/contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol#L351) + State is changed at: `totalCollateral -= totalCollateralClaimed` ```solidity treasuryReservesVault.repay(templeToken, totalCollateralClaimed, address(tlcStrategy)); ``` - Found in contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol [Line: 394](../tests/2024-07-templegold/protocol/contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol#L394) + State is changed at: `treasuryReservesVault = ITreasuryReservesVault(_trv)` ```solidity daiToken.approve(previousTrv, 0); ``` - Found in contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol [Line: 397](../tests/2024-07-templegold/protocol/contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol#L397) + State is changed at: `treasuryReservesVault = ITreasuryReservesVault(_trv)` ```solidity address _trv = address(tlcStrategy.treasuryReservesVault()); ```