There are contracts that do not emit events on critical places like adding liquidity, setting state variables, doing withdrawals... this is bad because 1) there are discrepancies between contracts, as some are covered whilst others are not 2) to verify changes on state variables, users/admins will need to read the value directly from the blockchain, which is a burden and 3) many automatic tools that monitors the contracts deployed and the blockchain rely on them, so they won't be as useful at stopping/detecting threats
Proof of Concept
Contracts like RdpxV2Core do follow bests practices and emits events whenever is possible. However, many others (the mentioned above) do not. Some examples:
Constructors like the one at UniV3LiquidityAmo do not emit any, so for the deployer/users to verify the correctness of the deployment (AKA state variables rdpx and rdpxV2Core) will need to read the blockchain directly (using cast, for example)
In RpdxDecayingBonds there is an emergencyWithdraw event which is not used in his respective function, whilst its equivalent in UniV2LiquidityAmo does emit LogEmergencyWithdraw
UniV3LiquidityAmo::addLiquidity does not emit any whilst UniV3LiquidityAmo::removeLiquidity does emit two. Its equivalent on UniV2LiquidityAmo does emit one, too
RdpxV2Core::setAddresses does emit one event logging the changes whilst its equivalent on UniV2LiquidityAmo does not
...
Tools Used
Manual analysis
Recommended Mitigation Steps
Consider emitting events after sensitive changes take place (including in the constructor), to facilitate tracking and notify off-chain clients following the contracts’ activity. Some of them are even defined in the contract but not used, so consider using them.
Lines of code
https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/reLP/ReLPContract.sol#L163 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/reLP/ReLPContract.sol#L179 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/reLP/ReLPContract.sol#L194 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/reLP/ReLPContract.sol#L307 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L214 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L205 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L196 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L187 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L182 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/decaying-bonds/RdpxDecayingBonds.sol#L64 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/decaying-bonds/RdpxDecayingBonds.sol#L107 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/decaying-bonds/RdpxDecayingBonds.sol#L145 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV3LiquidityAmo.sol#L89 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV3LiquidityAmo.sol#L133 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV3LiquidityAmo.sol#L152 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV3LiquidityAmo.sol#L211 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV3LiquidityAmo.sol#L308 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV2LiquidityAmo.sol#L117 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV2LiquidityAmo.sol#L135
Vulnerability details
Impact
NOTE -> Medium severity as here, here and here
There are contracts that do not emit events on critical places like adding liquidity, setting state variables, doing withdrawals... this is bad because 1) there are discrepancies between contracts, as some are covered whilst others are not 2) to verify changes on state variables, users/admins will need to read the value directly from the blockchain, which is a burden and 3) many automatic tools that monitors the contracts deployed and the blockchain rely on them, so they won't be as useful at stopping/detecting threats
Proof of Concept
Contracts like
RdpxV2Core
do follow bests practices and emits events whenever is possible. However, many others (the mentioned above) do not. Some examples:UniV3LiquidityAmo
do not emit any, so for the deployer/users to verify the correctness of the deployment (AKA state variablesrdpx
andrdpxV2Core
) will need to read the blockchain directly (usingcast
, for example)RpdxDecayingBonds
there is an emergencyWithdraw event which is not used in his respective function, whilst its equivalent inUniV2LiquidityAmo
does emitLogEmergencyWithdraw
UniV3LiquidityAmo::addLiquidity
does not emit any whilstUniV3LiquidityAmo::removeLiquidity
does emit two. Its equivalent onUniV2LiquidityAmo
does emit one, tooRdpxV2Core::setAddresses
does emit one event logging the changes whilst its equivalent onUniV2LiquidityAmo
does notTools Used
Manual analysis
Recommended Mitigation Steps
Consider emitting events after sensitive changes take place (including in the constructor), to facilitate tracking and notify off-chain clients following the contracts’ activity. Some of them are even defined in the contract but not used, so consider using them.
Assessed type
Other