yAxis contest - hickuphh3's results

Handle

hickuphh3

Vulnerability details

Impact

There are 2 cases that can lead to permanently locked funds.

Deployment with incorrect admin

Should the adapter be deployed with the admin specified as an address that isn't the Alchemist contract, deposits through the Alchemist contract will work, but withdrawals will revert.

Deposits directly via the adapter

A user (non-admin) that chooses to deposit into the yAxis vault directly through the adapter will be unable to withdraw his funds.

Recommended Mitigation Steps

deposit() should have the onlyAdmin modifier as well.

function deposit(uint256 _amount) external override onlyAdmin {
  vault.deposit(_amount);
}

Handle

hickuphh3

Vulnerability details

Impact

FixedPointMath is imported, but isn't used by the adapter and is therefore redundant.

Recommended Mitigation Steps

The import can be removed.

import {FixedPointMath} from '../libraries/FixedPointMath.sol';

Handle

hickuphh3

Vulnerability details

Impact

The vault, admin and vault's token variables can be made immutable. The constructor method will have to be modified to workaround the limitation of being unable to read immutable variables during contract creation time, but it will be worthwhile to do so for the gas savings made.

Recommended Mitigation Steps

/// @dev The vault that the adapter is wrapping.
IVault public immutable vault;

/// @dev The address which has admin control over this contract.
address public immutable admin;

IDetailedERC20 public immutable token;

constructor(IVault _vault, address _admin) public {
  vault = _vault;
  admin = _admin;
	IDetailedERC20 _token = IDetailedERC20(_vault.getToken());
	token = _token;
	_token.safeApprove(address(_vault), uint256(-1));
}

// token() view function becomes redundant
// function token() external view override returns (IDetailedERC20) {
//   return IDetailedERC20(vault.getToken());
// }

function withdraw(address _recipient, uint256 _amount) external override onlyAdmin {
  vault.withdraw(_tokensToShares(_amount));
  token.safeTransfer(_recipient, _amount);
}

/// @dev Updates the vaults approval of the token to be the maximum value.
function updateApproval() external {
	token.safeApprove(address(vault), uint256(-1));
}