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

In AuraMinter.sol the comment defines that tokens should not be able to unlock until "after 4 years have passed" #363

Closed
itsmetechjay opened this issue May 25, 2022 · 3 comments
Labels
bug Something isn't working disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) duplicate This issue or pull request already exists QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax

Comments

@itsmetechjay
Copy link
Contributor

In AuraMinter.sol the comment defines that tokens should not be able to unlock until "after 4 years have passed" ( https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraMinter.sol#L9-L10).

Impact

The impact of this fault, would be that the dao address could mint prior to the supposed '4' year lockup as stated by the comment

 * @notice  Wraps the AuraToken minterMint function and protects from inflation until
 *          4 years have passed.

(https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraMinter.sol#L9-L10)

Currently, the code checks the inception block_timestamp and sets a mint lock to + 156 weeks (3 years)
-> 52 weeks in 1 year, 156/52=3.

    constructor(address _aura, address _dao) Ownable() {
        aura = AuraToken(_aura);
        _transferOwnership(_dao);
        inflationProtectionTime = block.timestamp + 156 weeks;
    }

(https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraMinter.sol#L23)

So currently, the unlock and inflation will begin in the first week of the 4th year:

Proof of Concept

date = 22/05/2022 (1st year currently in progress),
date + 52 weeks = 21/05/2023 (one year has passed, now into second year),
date + 104 weeks = 19/05/2024 (two years has passed, now into third year),
date + 156 weeks = 18/05/2025 (three years have passed, now into fourth year),
date + 208 weeks = 17/05/2026 (4 years have now 'passed').

At this point, minter can mint Aura tokens, whilst users may believe it could not be done for 1 more year hence by calling:

    /**
     * @dev Mint function allows the owner of the contract to inflate AURA post protection time
     * @param _to Recipient address
     * @param _amount Amount of tokens
     */
    function mint(address _to, uint256 _amount) external onlyOwner {
        require(block.timestamp > inflationProtectionTime, "Inflation protected for now");
        aura.minterMint(_to, _amount);
    }
}

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraMinter.sol#L26-L35

Mitigation steps

Discussion with the team confirmed there is a mismatch between their expectation, and the comment - they were expecting 3 year lockup, not 4 years.

Mitigation step would be to change the comment to reflect this notion.

@itsmetechjay itsmetechjay added bug Something isn't working 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels May 25, 2022
@itsmetechjay
Copy link
Contributor Author

This was originally included as part of #172 but the warden asked for it to be elevated to a Medium status via help desk request.

itsmetechjay added a commit that referenced this issue May 25, 2022
Creating this json to link to the newly created issue #363
@0xMaharishi 0xMaharishi added duplicate This issue or pull request already exists disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) labels May 28, 2022
@dmvt
Copy link
Collaborator

dmvt commented Jun 25, 2022

Downgrading this back to QA. It is a textbook comment issue.

@dmvt dmvt added QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Jun 25, 2022
@dmvt
Copy link
Collaborator

dmvt commented Jul 8, 2022

Grouping this with the warden’s QA report, #172

@dmvt dmvt closed this as completed Jul 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) duplicate This issue or pull request already exists QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
Projects
None yet
Development

No branches or pull requests

3 participants