search

code-423n4 / 2021-12-nftx-findings

0 stars 0 forks source link

Cached lpStaking and inventoryStaking in Zap contracts #214

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

pauliax

Vulnerability details

Impact

NFTXMarketplaceZap and NFTXStakingZap cache addresses of lpStaking and inventoryStaking in the constructor:

  lpStaking = INFTXLPStaking(INFTXFeeDistributor(INFTXVaultFactory(_nftxFactory).feeDistributor()).lpStaking());

  lpStaking = INFTXLPStaking(INFTXSimpleFeeDistributor(INFTXVaultFactory(_nftxFactory).feeDistributor()).lpStaking());
  inventoryStaking = INFTXInventoryStaking(INFTXSimpleFeeDistributor(INFTXVaultFactory(_nftxFactory).feeDistributor()).inventoryStaking());

However, these values might be updated in the fee distributor contract (functions setLPStakingAddress, setInventoryStakingAddress) or even the fee distributor itself can change (setFeeDistributor). Now you will need to deploy new Zap contracts if any of these values change.

While I am not sure about the intentions, I wanted to point out this potential issue, and you can clarify if that's an issue or intended behavior.

Recommended Mitigation Steps

To fix this it would increase gas costs but you should query an up to date values whenever you need them.

0xKiwi commented 2 years ago

The rationale is that the zaps are easily changeable if we need to make modifications or deploy.