code4rena-2024-03-dittoeth-h05

[H-05] Flawed if check causes inaccurate tracking of the protocol’s ercDebt and collateral

보고서

Summary

|| 대신 &&를 잘못 사용하여 접근 제한을 제대로 하지 못한다. 이로 인해 아무나 청산 후 남은 담보를 꺼내갈 수 있다.

Keyword

access control, bug

Vulnerability

• facets/RedemptionFacet.sol#L361

RedemptionFacet.claimRemainingCollateral 함수는 청산당한 숏이 (CR은 낮지만) 과담보 상태일 때 등 담보가 남아있을 때 이를 꺼내가는 함수이다.

원래는 숏의 주인만 꺼내가기를 의도했지만 조건문에서 || 대신 &&를 잘못 사용하여 숏터가 msg.sender 가 아니더라도 claimIndex 와 id 만 맞추면 아무나 꺼내갈 수 있다.

    function claimRemainingCollateral(address asset, address redeemer, uint8 claimIndex, uint8 id)
        external
        isNotFrozen(asset)
        nonReentrant
    {
        STypes.AssetUser storage redeemerAssetUser = s.assetUser[asset][redeemer];
        if (redeemerAssetUser.SSTORE2Pointer == address(0)) revert Errors.InvalidRedemption();
        if (redeemerAssetUser.timeToDispute > LibOrders.getOffsetTime()) revert Errors.TimeToDisputeHasNotElapsed();
 
        // @dev Only need to read up to the position of the SR to be claimed
        MTypes.ProposalData[] memory decodedProposalData =
            LibBytes.readProposalData(redeemerAssetUser.SSTORE2Pointer, claimIndex + 1);
        MTypes.ProposalData memory claimProposal = decodedProposalData[claimIndex];
 
@>      if (claimProposal.shorter != msg.sender && claimProposal.shortId != id) revert Errors.CanOnlyClaimYourShort();
 
        STypes.Asset storage Asset = s.asset[asset];
        _claimRemainingCollateral({asset: asset, vault: Asset.vault, shorter: msg.sender, shortId: id});
    }

Impact

아무나 숏 청산 후 남은 담보를 꺼내갈 수 있다.

Mitigation

조건문에서 || 를 사용한다.

+   if (claimProposal.shorter != msg.sender || claimProposal.shortId != id) revert Errors.CanOnlyClaimYourShort();
-   if (claimProposal.shorter != msg.sender && claimProposal.shortId != id) revert Errors.CanOnlyClaimYourShort();

---

tags: bughunting, dittoeth, smart contract, solidity, access control vulnerability, severity high