hats-finance / Convergence---Convex-integration-0xb3df23e155b74ad2b93777f58980d6727e8b40bb

0 stars 1 forks source link

Missing Proper NATSpec across many functions of many contracts in scope #54

Open hats-bug-reporter[bot] opened 7 months ago

hats-bug-reporter[bot] commented 7 months ago

Github username: @0xumarkhatab Twitter username: 0xumarkhatab Submission hash (on-chain): 0x34dfd0b14461cad13c070e7c5de7e5e7fd112b7044b590a154f0279a839e563a Severity: medium

Description: Description\

Most of the contracts and functions in the audited code base lack documentation. This hinders reviewers’ understanding of the code’s intention, which is fundamental to correctly assess not only security, but also correctness. Additionally, docstrings improve readability and ease maintenance. They should explicitly explain the purpose or intention of the functions, the scenarios under which they can fail, the roles allowed to call them, the values returned and the events emitted.

Consider thoroughly documenting all functions (and their parameters) that are part of the contracts’ public API. Functions implementing sensitive functionality, even if not public, should be clearly documented as well. When writing docstrings, consider following the Ethereum Natural Specification Format (NatSpec).

Additionally, the OpenZeppelin team found a notable lack of comments throughout the audited code. Well-commented code not only improves audit speed and depth, but it also helps to reveal what developer intentions may be and thus helps identify issues of misalignment between intention and implementation. Without comments, identifying issues in the code is much more difficult. Aside from benefiting auditors, code comments also benefit future developers and users, by clearly defining the functionality of the code and by reducing the risk of bugs.

Consider thoroughly commenting the existing code, and adding regular commenting to the software development process. We specifically recommend commenting every line of assembly code and commenting all complex math operations.

Some are listed here for reference .

some contains documentation but missing naspec of one or tow or all params .

CvxAssetStakingService.sol

 function initialize(
        IERC20 _asset,
        IERC20 _cvxAsset,
        ICvxAssetWrapper _cvxAssetWrapper,
        ICrvPoolPlain _curvePool,
        IAssetDepositor _assetDepositor,
        string memory _symbol
    ) external initializer {

 function _deposit(
        uint256 tokenId,
        uint256 amount,
        INPUT_TOKEN_TYPE inputTokenType,
        uint256 minAmountOut,
        bool isLock,
        bool isStake,
        bool isEthDeposit
    ) internal {

function _mintOrSwapToCvxAsset(
        uint256 amount,
        uint256 minAmountOut,
        address _stakerBuffer,
        bool isLock,
        bool isEthDeposit
    ) internal returns (uint256) {

CvxRewardDistributor.sol

  function initialize(ICvx1 _cvx1) external initializer {

CVXAssetStakeBuffer.sol

    function pullRewards(address processor) external returns (ICommonStruct.TokenAmount[] memory) {

CvgCvxStakingPositionService

 struct DepositCvxData {
        uint256 amount;
        uint256 minAmountOut;
    }

    enum TOKEN_TYPE {
        cvgCVX,
        CVX1,
        CVX
    }

 function _deposit(
        uint256 tokenId,
        uint256 cvgCvxAmount,
        DepositCvxData memory cvxData,
        bool isEthDeposit
    ) internal {

  missing spec of isETHDeposit

 function _convertCvxToCvgCvx(
        uint256 cvxAmount,
        uint256 minCvgCvxAmountOut,
        bool isEthDeposit
    ) internal returns (uint256) {

CVX1

 function initialize(
        string memory _name,
        string memory _symbol,
        ICvxRewardPool _cvxRewardPool
    ) external initializer {

 function mint(address receiver, uint256 amount) external {

function withdraw(uint256 amount, address receiver) external {
  function recoverRewards(IERC20[] calldata erc20s) external {

CvxConvergenceLocker

  struct RewardConfiguration {
        IERC20 token;
        uint48 processorFees;
        uint48 podFees;
    }
function initialize(
        string memory _name,
        string memory _symbol,
        IDelegateRegistry _cvxDelegateRegistry,
        ICvx1 _cvx1,
        RewardConfiguration[] calldata _rewardTokensConfiguration
    ) external initializer {

   function setCvxDelegateRegistry(IDelegateRegistry delegateRegistry) external onlyOwner {
        cvxDelegateRegistry = delegateRegistry;
    }

function setRewardTokensConfiguration(
        RewardConfiguration[] calldata _rewardTokensConfiguration
    ) external onlyOwner {

Attack Scenario\ Please check the detail section

Attachments

  1. Proof of Concept (PoC) File N/A

  2. Revised Code File (Optional) N/A

kakarottosama commented 7 months ago

since when missing NatSpec is a medium issue? it's not even low, it's just informational

0xumarkhatab commented 7 months ago

Oh it was a mistake , it is low however because missing a lot of information on code natspec is similar to missing docs for code which can cause confusion about what code does to the layman who wants to use the protocol and wants ti just get hang of the logic .