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

timelockMint In TimelockRewardDistributionTokenImpl Does Not Ensure Mint Is Greater Than Zero #64

Open
code423n4 opened this issue Dec 19, 2021 · 1 comment
Labels
1 (Low Risk) Assets are not at risk. State handling, function incorrect as to spec, issues with comments bug Something isn't working sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons

Comments

@code423n4
Copy link
Contributor

Handle

leastwood

Vulnerability details

Impact

The timelockMint function attempts to mint a token amount via some timelock period. However, minting an amount = 0 will cause the caller's tokens to be timelocked again.

Proof of Concept

https://github.com/code-423n4/2021-12-nftx/blob/main/nftx-protocol-v2/contracts/solidity/token/TimelockRewardDistributionTokenImpl.sol#L100-L105

function timelockMint(address account, uint256 amount, uint256 timelockLength) public onlyOwner virtual {
  uint256 timelockFinish = block.timestamp + timelockLength;
  timelock[account] = timelockFinish;
  emit Timelocked(account, amount, timelockFinish);
  _mint(account, amount);
}

Tools Used

Manual code review.

Recommended Mitigation Steps

Consider disallowing amount = 0 from being used in timelockMint. The following require statement can be used to enforce this behaviour.

require(amount > 0, "Cannot mint zero amount");
@code423n4 code423n4 added 1 (Low Risk) Assets are not at risk. State handling, function incorrect as to spec, issues with comments bug Something isn't working labels Dec 19, 2021
code423n4 added a commit that referenced this issue Dec 19, 2021
@0xKiwi
Copy link
Collaborator

0xKiwi commented Dec 31, 2021

Fair find. Confirming.

@0xKiwi 0xKiwi added sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons and removed sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") labels Dec 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 (Low Risk) Assets are not at risk. State handling, function incorrect as to spec, issues with comments bug Something isn't working sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons
Projects
None yet
Development

No branches or pull requests

2 participants