XDEFI contest - harleythedog's results

Handle

harleythedog

Vulnerability details

Impact

The function updateDistribution is called to update the value of _pointsPerUnit based on new XDEFI that has been transferred to the contract since the last time it was called. However, if XDEFI is transferred into the contract and unlock or relock are called before updateDistribution, then the new XDEFI will be locked in the contract forever and it will not contribute to _pointsPerUnit. This is because unlock and relock call _updateXDEFIBalance, which updates distributableXDEFI but does not update _pointsPerUnit.

Since there is no function to rescue locked tokens from the contract (which makes sense, otherwise the admin would have too much power), this XDEFI is locked into the contract forever, so this is a high severity finding.

Proof of Concept

Suppose that Alice transfers 100 XDEFI into the contract, but decides to let someone else call updateDistribution to increase _pointsPerUnit (or perhaps Alice forgets that this step even exists). Now, suppose that Bob calls unlock on his position. At the end of the function call, _updateXDEFIBalance is called. This function will recognize the new 100 XDEFI that has been transferred into the contract, so distributableXDEFI will increase by 100. However, this will not update _pointsPerUnit. A subsequent call to updateDistribution will not increase _pointsPerUnit, since updateDistribution updates _pointsPerUnit based on the change in distributableXDEFI, and in this case there will be 0 change since distributableXDEFI was already increased.

Tools Used

Manual inspection.

Recommended Mitigation Steps

At the end of unlock and relock, call updateDistribution instead of _updateXDEFIBalance.

The warden has added the following note about mitigating steps:

Instead of calling updateDistribution at the end of unlock and relock, a better fix is to call updateDistribution at the START of unlock and relock. If updateDistribution was called at the end instead, I believe _pointsPerUnit may be updated incorrectly. Calling updateDistribution at the start of the function definitely would fix my finding above.