FairSide contest - hickuphh3's results

Handle

hickuphh3

Vulnerability details

Impact

The updateVestedTokens() increases the amount of tokens to be vested for a beneficiary. There is no access restriction to the function.

The intended total vesting duration is 30 months with a 12-month cliff where 5% is immediately unlocked, and the remaining 95% linearly vested over the next 18 months. Hence, a beneficiary can prematurely unlock the full amount immediately after the 12-month cliff by specifying calling the function again with 19x the amount.

Example

1. Alice calls FSD.mintHatch(), where the computed mintAmount is 100 FSD tokens. The vesting contract is created with 100 FSD tokens vested.
2. Alice calls updateVestedTokens(ALICE_ADDRESS, 1900e18) on the created vesting contract with the following parameters. The amount in the contract is now 100 + 1900 = 2000 FSD tokens.
3. Alice calls calculateVestingClaim() where her tx gets included in a block such that block.timestamp == start.add(CLIFF). Then, the unlock amount vestedAmount = amount.mul(5) / 100; would be 2000 * 5 / 100 = 100, the entire FSD balance being unlocked instead of the expected 5% of 100 = 5 FSD tokens.

Realistically, a user would likely call with a slightly smaller multiplier due to unpredictable block times. Nevertheless, they would be able to prematurely unlock most of the vested amount.

Recommended Mitigation Steps

Restrict the updateVestedTokens() function to only be callable by the FSD token contract.

function updateVestedTokens(address _user, uint256 _amount)
	external
	override
	onlyFsd
{
	// Note: checks below are redundant
  require(
      _user == beneficiary,
      "FSDVesting::updateVestedTokens: Only beneficiary can update the vested amount"
  );
  require(
      amount != 0,
      "FSDVesting::updateVestedTokens: Amount cannot be zero"
  );

  amount = amount.add(_amount);
}

...

/**
* @dev Throws if called by any account other than the FSD token contract
*/
modifier onlyFsd() {
	require(
    address(fsd) == msg.sender,
		"FSDVesting::updateVestedTokens: Only FSD can update the vested amount"
  );
	_;
}

Handle

hickuphh3

Vulnerability details

Impact

The claiming of staking and governance tributes for the a beneficiary's vested tokens should be no different than other users / EOAs. However, the claimTribute() and claimGovernanceTribute() are missing the actual claiming calls to the corresponding functions of the FSD token contract. As a result, the accrued rewards are taken from the beneficiary's vested token while not claiming (replenishing) from the FSD token contract.

Recommended Mitigation Steps

In addition to what has been mentioned above, the internal accounting for claimedTribute states can be removed because they are already performed in the FSD token contract.

// TODO: Remove _claimedTribute and _claimedGovernanceTribute mappings

/**
* @dev Allows claiming of staking tribute by `msg.sender` during their vesting period.
* It updates the claimed status of the vest against the tribute
* being claimed.
*
* Requirements:
* - claiming amount must not be 0.
*/
function claimTribute(uint256 num) external onlyBeneficiary {
    uint256 tribute = fsd.availableTribute(num);
    require(tribute != 0, "FSDVesting::claimTribute: No tribute to claim");
		fsd.claimTribute(num);
    fsd.safeTransfer(msg.sender, tribute);
    emit TributeClaimed(msg.sender, tribute);
}

/**
* @dev Allows claiming of governance tribute by `msg.sender` during their vesting period.
* It updates the claimed status of the vest against the tribute
* being claimed.
*
* Requirements:
* - claiming amount must not be 0.
*/
function claimGovernanceTribute(uint256 num) external onlyBeneficiary {
  uint256 tribute = fsd.availableGovernanceTribute(num);
  require(
    tribute != 0,
    "FSDVesting::claimGovernanceTribute: No governance tribute to claim"
  );
  fsd.claimGovernanceTribute(num);
  fsd.safeTransfer(msg.sender, tribute);
  emit GovernanceTributeClaimed(msg.sender, tribute);
}