code-423n4 / 2024-02-spectra-findings

4 stars 2 forks source link

Wrong event argument #17

Closed c4-bot-3 closed 8 months ago

c4-bot-3 commented 8 months ago

Lines of code

https://github.com/perspectivefi/spectra-core/blob/main/src/tokens/PrincipalToken.sol#L72 https://github.com/perspectivefi/spectra-core/blob/main/src/tokens/PrincipalToken.sol#L370 https://github.com/perspectivefi/spectra-core/blob/main/src/tokens/PrincipalToken.sol#L378 https://github.com/perspectivefi/spectra-core/blob/main/src/tokens/PrincipalToken.sol#L848-L859

Vulnerability details

Impact

The event YieldClaimed is defined as follows:

event YieldClaimed(address indexed owner, address indexed receiver, uint256 indexed yieldInIBT);

It is emitted in _claimYield function with receiver equal to msg.sender even if this not the receiver.

emit YieldClaimed(msg.sender, msg.sender, yieldInIBT);

_claimYield is only used in those 2 functions:

    /** @dev See {IPrincipalToken-claimYield}. */
    function claimYield(address _receiver) public override returns (uint256 yieldInAsset) {
        uint256 yieldInIBT = _claimYield();
        if (yieldInIBT != 0) {
            yieldInAsset = IERC4626(ibt).redeem(yieldInIBT, _receiver, address(this));
        }
    }

    /** @dev See {IPrincipalToken-claimYieldInIBT}. */
    function claimYieldInIBT(address _receiver) public override returns (uint256 yieldInIBT) {
        yieldInIBT = _claimYield();
        if (yieldInIBT != 0) {
            IERC20(ibt).safeTransfer(_receiver, yieldInIBT);
        }
    }

In the case where _receiver differs from msg.sender, it will emit an event with wrong parameters.

Tools Used

Manual review

Recommended Mitigation Steps

The event should be emitted in the claimYield and claimYieldInIBT functions directly, and deleted in the _claimYield function.

    /** @dev See {IPrincipalToken-claimYield}. */
    function claimYield(address _receiver) public override returns (uint256 yieldInAsset) {
        uint256 yieldInIBT = _claimYield();
        emit YieldClaimed(msg.sender, _receiver, yieldInIBT);
        if (yieldInIBT != 0) {
            yieldInAsset = IERC4626(ibt).redeem(yieldInIBT, _receiver, address(this));
        }
    }

    /** @dev See {IPrincipalToken-claimYieldInIBT}. */
    function claimYieldInIBT(address _receiver) public override returns (uint256 yieldInIBT) {
        yieldInIBT = _claimYield();
        emit YieldClaimed(msg.sender, _receiver, yieldInIBT);
        if (yieldInIBT != 0) {
            IERC20(ibt).safeTransfer(_receiver, yieldInIBT);
        }
    }

Assessed type

Other

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as sufficient quality report

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as primary issue

c4-sponsor commented 8 months ago

yanisepfl marked the issue as disagree with severity

yanisepfl commented 8 months ago

This is a good catch indeed. However, we disagree with the issue's severity and consider it as a QA report.

c4-sponsor commented 8 months ago

yanisepfl (sponsor) disputed

c4-judge commented 8 months ago

JustDravee marked the issue as unsatisfactory: Invalid