hats-finance / VMEX-0xb6861bdeb368a1bf628fc36a36cec62d04fb6a77

LP token lending protocol
MIT License
2 stars 4 forks source link

AURA claims on withdraw, HarvestedReward(stakingContract) is not emitted (low severity) #3

Open ksyao2002 opened 1 year ago

ksyao2002 commented 1 year ago

Communication channel: GalloDaSballo (discord)

https://github.com/VMEX-finance/vmex/blob/790ed430e45874e66f6ce461457d1e511561590d/packages/contracts/contracts/protocol/incentives/ExternalRewardDistributor.sol#L119-L120

      emit HarvestedReward(stakingContract);

Is not emitted but withdrawing from AURA claims rewards

Attack Scenario\

The event is emitted on Harvest, however, Aura Withdrawals have the claim parameter set to true this means those rewards will be harvested without triggering an event

POC

      require(IAuraRewardPool(stakingContract).withdrawAndUnwrap(amount, true), "aura unstaking failed");

As yo can see

    function withdrawAndUnwrap(uint256 amount, bool claim) public returns(bool){
        _withdrawAndUnwrapTo(amount, msg.sender, msg.sender);
        //get rewards too
        if(claim){
            getReward(msg.sender,true);
        }
        return true;
    }

https://optimistic.etherscan.io/address/0x9f43f726df654e033b04c39989af90ab44875feb#code#F14#L260

Will trigger a claim

fico23 commented 1 year ago

This is off chain tracked in the subgraph, so we dont have to explicitly handle it in the contract @ksyao2002

GalloDaSballo commented 1 year ago

This should be considered as informational because while you can say you would track the even via theGraph, the finding shows that the event would not fire, making the event emission itself unnecessary (at that point just use theGraph on the Aura Rewards Contract and filter for your own contract as the recipient)

That's a legitimate way to fix that makes the even emission unnecessary / confirms the finding as informational