Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inadequate Event Emission in Critical Functions Reduces Transparency and Auditing Capabilities #7

Open
CipherHawk0 opened this issue Oct 14, 2024 · 0 comments

Comments

@CipherHawk0
Copy link

Summary

The lack of comprehensive event emissions in critical functions will cause a low impact on the protocol's transparency as certain state changes and operations are not logged, hindering effective monitoring and auditing.

Root Cause

In SolidlyV2AMO.sol, while the contract emits events for major operations like adding liquidity and removing liquidity, not all critical state changes and function executions have corresponding detailed event emissions. For example, the setParams function emits a ParamsSet event but does not capture all parameter changes or internal state modifications comprehensively.

Relevant Code Snippet:

function setParams(
    uint256 boostMultiplier_,
    uint24 validRangeWidth_,
    uint24 validRemovingRatio_,
    uint256 boostLowerPriceSell_,
    uint256 boostUpperPriceBuy_,
    uint256 boostSellRatio_,
    uint256 usdBuyRatio_
) public override onlyRole(SETTER_ROLE) {
    // Parameter validation...
    boostMultiplier = boostMultiplier_;
    validRangeWidth = validRangeWidth_;
    validRemovingRatio = validRemovingRatio_;
    boostLowerPriceSell = boostLowerPriceSell_;
    boostUpperPriceBuy = boostUpperPriceBuy_;
    boostSellRatio = boostSellRatio_;
    usdBuyRatio = usdBuyRatio_;
    emit ParamsSet(
        boostMultiplier,
        validRangeWidth,
        validRemovingRatio,
        boostLowerPriceSell,
        boostUpperPriceBuy,
        boostSellRatio,
        usdBuyRatio
    );
}

Internal pre-conditions

1-Critical functions are executed that modify key state variables without emitting detailed events.
2-Internal state changes within functions do not trigger additional event emissions.

External pre-conditions

1-External monitoring tools rely on event emissions to track contract activities.
2-Users and auditors depend on comprehensive event logs to verify the correctness and security of contract operations.

Attack Path

1-The protocol performs critical operations that modify state variables without emitting detailed events.
2-An observer or auditor attempts to monitor contract activities by tracking events.
3-Due to incomplete event emissions, certain state changes and operations go unlogged.
4-This lack of transparency makes it difficult to verify the integrity of contract operations and detect potential anomalies or unauthorized actions.

Impact

The protocol experiences a low risk of reduced transparency and auditing capabilities, making it harder to monitor contract operations effectively and conduct thorough audits, potentially leading to undetected issues or unauthorized activities.

PoC

// SPDX-License-Identifier: MIT
pragma solidity 0.8.19;

import "./SolidlyV2AMO.sol";

contract MonitoringContract {
    SolidlyV2AMO amo;

    constructor(address amoAddress) {
        amo = SolidlyV2AMO(amoAddress);
    }

    function monitorSetParams() public view returns (bool) {
        // Attempt to monitor parameter changes via events
        // Notice that only ParamsSet is emitted without detailed parameter breakdown
        // Internal state changes are not captured
        return true;
    }
}

Mitigation

1-Emit Detailed Events in All Critical Functions:
Ensure that every function modifying significant state variables or performing critical operations emits events capturing all relevant parameters and outcomes.

event ParamsSetDetailed(
    uint256 boostMultiplier,
    uint24 validRangeWidth,
    uint24 validRemovingRatio,
    uint256 boostLowerPriceSell,
    uint256 boostUpperPriceBuy,
    uint256 boostSellRatio,
    uint256 usdBuyRatio
);

function setParams(
    uint256 boostMultiplier_,
    uint24 validRangeWidth_,
    uint24 validRemovingRatio_,
    uint256 boostLowerPriceSell_,
    uint256 boostUpperPriceBuy_,
    uint256 boostSellRatio_,
    uint256 usdBuyRatio_
) public override onlyRole(SETTER_ROLE) {
    // Parameter validation...
    boostMultiplier = boostMultiplier_;
    validRangeWidth = validRangeWidth_;
    validRemovingRatio = validRemovingRatio_;
    boostLowerPriceSell = boostLowerPriceSell_;
    boostUpperPriceBuy = boostUpperPriceBuy_;
    boostSellRatio = boostSellRatio_;
    usdBuyRatio = usdBuyRatio_;
    emit ParamsSet(
        boostMultiplier,
        validRangeWidth,
        validRemovingRatio,
        boostLowerPriceSell,
        boostUpperPriceBuy,
        boostSellRatio,
        usdBuyRatio
    );
    emit ParamsSetDetailed(
        boostMultiplier,
        validRangeWidth,
        validRemovingRatio,
        boostLowerPriceSell,
        boostUpperPriceBuy,
        boostSellRatio,
        usdBuyRatio
    );
}

2-Standardize Event Logging:
Adopt a consistent event naming and parameter logging strategy across all functions to ensure uniformity and ease of monitoring.
3-Include Contextual Information:
Add additional contextual data to events, such as transaction origin, affected addresses, or previous state values, to provide a more comprehensive audit trail.
4-Regular Code Reviews:
Periodically review the contract’s codebase to ensure all critical operations are adequately logged with appropriate event emissions.
5-Leverage OpenZeppelin's Event Libraries:
Utilize established libraries or patterns for event emissions to ensure best practices and reduce the likelihood of omissions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant