code-423n4 / 2022-05-sturdy-findings

7 stars 3 forks source link

QA Report #80

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

QA Report

Table of Contents

Low Risk

[L-01]: Events not emitted for important state changes

Description

When changing state variables events are not emitted. Emitting events allows monitoring activities with off-chain monitoring tools.

Findings

YieldManager.setExchangeToken()\ YieldManager.registerAsset()\ YieldManager.setCurvePool()\ ConvexCurveLPVault.setConfiguration()\ CollateralAdapter.addCollateralAsset()

Recommended mitigation steps

Emit events for state variable changes.

[L-02]: Zero-address checks are missing

Description

Zero-address checks are a best-practice for input validation of critical address parameters. While the codebase applies this to most most cases, there are many places where this is missing in constructors and setters.

Impact: Accidental use of zero-addresses may result in exceptions, burn fees/tokens or force redeployment of contracts.

Findings

YieldManager.sol#L74

_assetsList[_assetsCount] = _asset;

ConvexCurveLPVault.sol#L41

curveLPToken = _lpToken;

Recommended mitigation steps

Add zero-address checks, e.g.:

require(_asset != address(0), "Zero-address");

[L-03]: Restrict ETH funds receivable by LidoVault to be only from Curve exchange

Description

Native ETH fund transfers into the LidoVault contract are only expected from the curveExchange contract. Hence, it would be good to restrict incoming ETH fund transfers to prevent accidental native fund transfers from other sources.

Findings

LidoVault.sol#L24

/**
  * @dev Receive Ether
  */
receive() external payable {}

Recommended mitigation steps

Modify the receive() function to only accept transfers from the wrapped token contract:

receive() external payable {
  require(msg.sender == address(curveExchange), 'Only curve exchange');
}
HickupHH3 commented 2 years ago

Low: L03 NC: L01, L02