OpenLeverage contest - pauliax's results

Handle

pauliax

Vulnerability details

Impact

function transferTo allows transferring amount from beneficiary to any address. However, 'to' is considered valid when it does not have an amount locked yet:

 function transferTo(address to, uint amount) external
 ...
   require(releaseVars[to].amount == 0, 'to is exist');

It locks this amount for releaseVars[beneficiary].endTime. Because the blockchain is public, a malicious actor could monitor the mempool, and crash any attempt of transferTo by frontrunning it and calling transferTo with the smallest fraction (dust) from his own address to the 'to' address, making it unavailable to receive new locks for some time (even 4 years is possible?).

Recommended Mitigation Steps

A few possible solutions would be to introduce a reasonable minimum amount to transfer or add a 2-step approval, where 'to' first have to approve the beneficiary.

Handle

pauliax

Vulnerability details

Impact

The condition should be _locked.amount > 0:

  require(_locked.amount >= 0, "Nothing to withdraw");

safe32 should be n <= 2 ** 32:

  require(n < 2 ** 32, errorMessage);

Handle

pauliax

Vulnerability details

Impact

There are variables that do not change so they can be marked as immutable to greatly improve the gas costs. Examples of such variables are: Airdrop.sol

  IERC20 public token;

Adminable.sol

  address payable public developer;

OLETokenLock.sol

  OLEToken public token;

Reserve.sol

  IERC20 public oleToken;

Please review all the state variables and apply immutable where possible.

Handle

pauliax

Vulnerability details

Impact

You should cache storage access to improve gas efficiency, e.g.: tranches[_trancheId] is accessed 5 times in function _claim. function acceptAdmin could first set local variable oldPendingAdmin and then compare against it.

Handle

pauliax

Vulnerability details

Impact

.length in a loop can be extracted into a variable and used where necessary to reduce the number of storage reads GovernorAlpha.sol

  for (uint i = 0; i < proposal.targets.length; i++)

Handle

pauliax

Vulnerability details

Impact

Assigned operations to constant variables are re-evaluated every time. See https://github.com/ethereum/solidity/issues/9232 Change from 'constant' to 'immutable'.

    // The EIP-712 typehash for the contract's domain
    bytes32 public constant DOMAIN_TYPEHASH = keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)");

    // The EIP-712 typehash for the delegation struct used by the contract
    bytes32 public constant DELEGATION_TYPEHASH = keccak256("Delegation(address delegatee,uint256 nonce,uint256 expiry)");

    // The EIP-712 typehash for the ballot struct used by the contract
    bytes32 public constant BALLOT_TYPEHASH = keccak256("Ballot(uint256 proposalId,bool support)");

    uint constant NEXT_OFFSET = ADDRESS_SIZE + FEE_SIZE;