Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing parameter validation #81

Open
code423n4 opened this issue Jun 23, 2021 · 8 comments
Open

Missing parameter validation #81

code423n4 opened this issue Jun 23, 2021 · 8 comments

Comments

@code423n4
Copy link
Contributor

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
Copy link
Member

ATokenYieldSource PR: pooltogether/aave-yield-source#19

@PierrickGT
Copy link
Member

BadgerYieldSource PR: pooltogether/badger-yield-source#6

@PierrickGT
Copy link
Member

SushiYieldSource PR: pooltogether/sushi-pooltogether#16

@PierrickGT
Copy link
Member

ControlledToken PR: pooltogether/pooltogether-pool-contracts#306

@PierrickGT
Copy link
Member

StakePrizePool PR: pooltogether/pooltogether-pool-contracts#314

@PierrickGT
Copy link
Member

@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
Copy link
Collaborator

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

@kamescg
Copy link
Collaborator

kamescg commented Jul 8, 2021

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants