search

code-423n4 / 2022-05-alchemix-findings

5 stars 2 forks source link

[gALCX.sol] Attacker can make the contract unusable when totalSupply is 0 #198

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-alchemix/blob/main/contracts-hardhat/gALCX.sol#L73-L76

Vulnerability details

Impact

An attacker can make the contract unusable when totalSupply is 0. Specifically, bumpExchangeRate function does not work correctly which results in making stake, unstake and migrateSource functions that do not work as expected.

Proof of Concept

Here are steps on how the gALCX contract can be unusable.

  1. gALCX contract is deployed

  2. The attacker sends the ALCX token to the deployed gALCX contract directly instead of using stake function so that the following balance variable has value.

https://github.com/code-423n4/2022-05-alchemix/blob/main/contracts-hardhat/gALCX.sol#L73-L75

uint balance = alcx.balanceOf(address(this));

if (balance > 0) {
  1. Since the ALCX token is given to the gALCX contract directly, totalSupply == 0 and alcx.balanceOf(address(this)) > 0 becomes true.

https://github.com/code-423n4/2022-05-alchemix/blob/main/contracts-hardhat/gALCX.sol#L76

exchangeRate += (balance * exchangeRatePrecision) / totalSupply;

  1. Non attackers try to call stake function, but bumpExchangeRate function fails because of (balance * exchangeRatePrecision) / totalSupply when totalSupply is 0.

  2. Owner cannot call migrateSource function since bumpExchangeRate will be in the same situation mentioned in the step4 above

Tools Used

Static code analysis

Recommended Mitigation Steps

Add handling when totalSupply is 0 but alcx.balanceOf(address(this)) is more than 0.

0xfoobar commented 2 years ago

Sponsor acknowledged

Given that the gALCX deployment has 412 unique tokenholders on mainnet, this series of events is extraordinarily unlikely to occur. But we will keep it in mind for future deployments.

0xleastwood commented 2 years ago

Nice find! Early stakers can DoS new contract deployments, making it impossible for other users to participate in the protocol. As this does not lead to lost funds and is recoverable through redeployment, I believe medium severity to be justified by the warden.