search

code-423n4 / 2021-09-defiprotocol-findings

1 stars 0 forks source link

licenseFee state variable not checked for maximum value (Basket.sol) #200

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

ye0lde

Vulnerability details

Impact

The "licenseFee" state variable (Basket.sol) is never checked for a maximum value. That unchecked value is then used in fee calculations that result in the number of tokens to be minted. A license fee can be set to possibly incorrect and very large values that are used in the mint calculations.

Proof of Concept

The "licenseFee" state variable is defined here: https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Basket.sol#L20

The "licenseFee" state variable is set (unverified) here: initialize - https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Basket.sol#L38

changeLicenseFee - https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Basket.sol#L157-L163

The "licenseFee" state variable is used just prior to and factors into minting here: handleFees - https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Basket.sol#L117-L121

Tools Used

Visual Studio Code

Recommended Mitigation Steps

Verify a reasonable maximum value for "licenseFee" just as the minimum value is verified.

The verification could happen here: initialize - https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Basket.sol#L38

changeLicenseFee - https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Basket.sol#L153

proposeBasketLicense (Factory.sol) https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Factory.sol#L74

GalloDaSballo commented 2 years ago

Agree with the finding and with severity, an unbounded licenseFee can cause undefined behaviour It seems to me that due to using uint this line right here could "underflow" into a massive number, due to underflow checks (solidity 0.8) that will cause a revert, potentially messing up with the protocol usage

Because the warden didn't specify an attack vector, I agree with a low severity