Not emitting event for important state changes

[code423n4]

Email address

simomonica1997@gmail.com

Handle

s1m0

Eth address

0x9b3E9e3E4a174d59279FC7cd268e035992412384

Vulnerability details

When changing state variables events are not emitted. PriceAware (https://github.com/code-423n4/marginswap/blob/main/contracts/PriceAware.sol):

• setPriceUpdateWindow
• setUpdateRate
• setUpdateMaxPegAmount
• setUpdateMinPegAmount Lending (https://github.com/code-423n4/marginswap/blob/main/contracts/Lending.sol):
• activateIssuer
• deactivateIssuer
• setLendingCap
• setLendingBuffer
• setHourlyYieldAPR
• setRuntimeWeights IncentiveDistribution (https://github.com/code-423n4/marginswap/blob/main/contracts/IncentiveDistribution.sol#L261):
• setTrancheShare
• initTranche IsolatedMarginTrading and CrossMarginTrading (https://github.com/code-423n4/marginswap/blob/main/contracts/IsolatedMarginTrading.sol - https://github.com/code-423n4/marginswap/blob/main/contracts/CrossMarginTrading.sol):
• setCoolingOffPeriod
• setLeveragePercent
• setLiquidationThresholdPercent

The events emitted by MarginRouter (https://github.com/code-423n4/marginswap/blob/main/contracts/MarginRouter.sol) don't have indexed parameter.

Proof of concept

-

Tools used

Manual analysis

Impact

The system doesn't record historical state changes.

Recommended mitigation steps

For set... function emit events with old and new value. For initTranche, event InitTranche(uint256 tranche, uint256 share) For activateIssuer, event ActivateIssuer(address issuer, address token) For deactivateIssuer, event DeactivateIssuer(address issuer) For events emitted by MarginRouter i would index the trader address to make it filterable.

[werg]

We may sprinkle in a few more events before launch, but in the interest of gas savings we try not to emit events for state that can be queried using view functions.

[zscole]

Reducing this from submitted rating of 2 (Med Risk) to 1 (Low Risk) since it presents no immediate risk to the security of the system, but could have implications on overall functionality.