code-423n4 / 2021-06-pooltogether-findings

0 stars 0 forks source link

Missing parameter validation #81

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

cmichel

Vulnerability details

Some parameters of functions are not checked for invalid values:

StakePrizePool.initialize: address _stakeToken not checked for non-zero or contract
ControlledToken.initialize: address controller not checked for non-zero or contract
PrizePool.withdrawReserve: address to not checked for non-zero, funds will be lost when sending to zero address
ATokenYieldSource.initialize: address _aToken, _lendingPoolAddressesProviderRegistry not checked for non-zero or contract
BadgerYieldSource.initialize: address badgerSettAddr, badgerAddr not checked for non-zero or contract
SushiYieldSource.constructor: address _sushiBar, _sushiAddr not checked for non-zero or contract

Impact

Wrong user input or wallets defaulting to the zero addresses for a missing input can lead to the contract needing to redeploy or wasted gas.

Recommended Mitigation Steps

Validate the parameters.

PierrickGT commented 3 years ago

ATokenYieldSource PR: https://github.com/pooltogether/aave-yield-source/pull/19

PierrickGT commented 3 years ago

BadgerYieldSource PR: https://github.com/pooltogether/badger-yield-source/pull/6

PierrickGT commented 3 years ago

SushiYieldSource PR: https://github.com/pooltogether/sushi-pooltogether/pull/16

PierrickGT commented 3 years ago

ControlledToken PR: https://github.com/pooltogether/pooltogether-pool-contracts/pull/306

PierrickGT commented 3 years ago

StakePrizePool PR: https://github.com/pooltogether/pooltogether-pool-contracts/pull/314

PierrickGT commented 3 years ago

@asselstine I'm not sure we want to check for non zero address in the PrizePool withdrawReserve function since this function is only callable by the Reserve and the owner of the Reserve contract. https://github.com/pooltogether/pooltogether-pool-contracts/blob/192429c808ad9714e9e05821386eb926150a009f/contracts/reserve/Reserve.sol#L32 https://github.com/pooltogether/pooltogether-pool-contracts/blob/4449bb2e4216511b7187b1ab420118c30af39eb7/contracts/prize-pool/PrizePool.sol#L473

asselstine commented 3 years ago

Yeah @PierrickGT I don't think the withdrawReserve needs to do the check. Many tokens reject on transfer to zero anyway.

kamescg commented 3 years ago

LGTM