Anyone can bypass the cooldown period and call harvest multiple times

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/vault/adapter/abstracts/AdapterBase.sol#L441 https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/vault/adapter/abstracts/AdapterBase.sol#L86

Vulnerability details

Impact

The last harvest is updated when the yearnAdapter is first initialised but this value is never updated again, even though it is called in deposit and withdraw. This value added to harvestCooldown returns the period in which one has to wait for another harvest. Even without a cooldown period, it is understood, that harvest will be called many times without restriction.But if one is set, no one should arbitrarily be able to call harvest until the new harvest period has started and ended. Currently, it is the old period that will always be added to whatever cooldown period set by the owner. This period can't be more than one day so the waiting period can always be bypassed.. I enquired from the dev(Veil), about the purpose of the cooldown period, it is understood that if there is a small amount of tokens within the adapter, one would not want to be selling those for more assets(which is what occurs during a harvest) . With this in mind, as it stands, the lack of update to lastHarvestin harvest would allow anyone to force even a small quantity of tokens to be sold. In the YearnAdapterTest.t.sol add this test :

 function test_Harvest()public {  

uint256 hc= adapter.harvestCooldown();    address owner =adapter.owner();  
console.log("cool down ", hc);    
vm.startPrank(owner);   

 uint256 cooldown =83600;  

 //even after cool down period anyone can call harvest without waiting as the lastHarvest timestamp is in the past  

adapter.setHarvestCooldown(cooldown);    

 vm.stopPrank();   

 uint256 hcAfter=adapter.harvestCooldown();    

console.log("cool down after ", hcAfter);    

vm.warp(cooldown);   

//Bob can call harvest as many times as he likes 

vm.startPrank(bob);   

for(uint256 i;i<5;i++){

 adapter.harvest();  

  }  

  } 

Recommended Mitigation Steps

The dev said that there is a merge issue but the fix is to update the lastHarvest so the cooldown period works as intended.

[c4-judge]

dmvt marked the issue as duplicate of #199

[c4-sponsor]

RedVeil marked the issue as sponsor confirmed

[c4-judge]

dmvt marked the issue as partial-50

[c4-judge]

dmvt marked the issue as full credit

[c4-judge]

dmvt marked the issue as satisfactory