HackMDLITRA Security Audit Report
GENERAL
Project Brief
| Title | Description |
|---|---|
| Client | Litra |
| Project name | Litra App |
| Timeline | 22-02-2023 - N/A |
| Initial commit | 26eb98aa2709707db72c9d9d065eba7f6616486c |
| Final commit | N/A |
Short Overview
Very chaotic, poorly commented code, no explanatory documentation. Many non-optimal decisions are made in the code, the review of which is beyond the scope of the audit.
Project Scope
The audit covered the following files:
| File name | Link |
|---|---|
| LA.sol | LA.sol |
| Voting.sol | Voting.sol |
| EmergencyAdminManaged.sol | EmergencyAdminManaged.sol |
| OwnershipAdminManaged.sol | OwnershipAdminManaged.sol |
| ParameterAdminManaged.sol | ParameterAdminManaged.sol |
| Stoppable.sol | Stoppable.sol |
| SimpleBurner.sol | SimpleBurner.sol |
| WrappedNFT.sol | WrappedNFT.sol |
| NftReceiver.sol | NftReceiver.sol |
Finding Severity breakdown
Each vulnerability discovered during the audit is classified based on their severity and likelihood and have the following classification:
| Severity | Description |
|---|---|
| Critical | Bugs leading to assets theft, funds locking, or any other issues that lead to fund loss. |
| High | Bugs that break contract core logic or lead to the contract state corruption, or any bugs that require manual recovery. |
| Medium | Bugs leading to partial failure of the contract minor logic under specific conditions or significant gas optimizations. |
| Informational | Bugs, suggestions, or optimizations that do not have a significant impact in terms of contract security. |
Based on the feedback received from the Client regarding the report, all issues assigned the following statuses:
| Status | Description |
|---|---|
| Fixed | Recommended fixes have been made to the project code and no longer affect its security. |
| Acknowledged | The Customer is aware of the finding. Recommendations for the finding are planned to be resolved in the future. |
Summary of findings
| Severity | # of Findings |
|---|---|
| CRITICAL | 1 |
| HIGH | 1 |
| MEDIUM | 2 |
| INFORMATIONAL | 5 |
Conclusion
| File name | Contract deployed on mainnet |
|---|---|
| N/A |
FINDINGS REPORT
CRITICAL
1. Unlimited mint of LA token, no checks on epoch supply.
Description
Despite having a complicated calculation model of mintable amounts, the mint method allows the minter to produce an unlimited number of tokens.
If this behavior is by design, then it is not at all obvious from the provided code.
Links to the code:
Recommendation
Add token mint limits according to the logic of mintable quantity calculation.
Status
NEW
Client's comments
comment here
HIGH
2. Creator has unlimited rights for mint/burn and can't be changed
Description
Due to the lack of project documentation, the deployment model is not clear. In case of the creator is an EOA address there is no way to chenge it address or transfer the role. At the same time creator has no limits to mint and burn. If this address is compromised, the users of the project are not protected in any way.
Link to the code: https://github.com/litrafi/litra-contract/blob/26eb98aa2709707db72c9d9d065eba7f6616486c/contracts/tokenize/WrappedNFT.sol#L10-L22
Recommendation
Create a separate method to set the creator's address, or use an role based access control. Add documented deployement plan and contract relationships.
Status
NEW
Client's comments
comment here
MEDIUM
3. Wrong totalSupply value in the event when creating the vote.
Description
At the moment of creating a vote, an incorrect value of token.totalSupply() is written to the StartVote event. It is necessary to write value at the snapshot block: token.totalSupplyAt(snapshotBlock), which is already stored in votingPower var.
Links to the code:
Recommendation
Replace event emit code with next one:
Status
NEW
Client's comments
comment here
4. Wrong totalSupply value in the event when creating the vote.
Description
Voting allows specifying a partial use of the user's vote power, which can be used to manipulate the voting process. For example, a user with significant voting power, votes with a very small part, thereby sabotaging the quorum.
Links to the code:
Recommendation
Enforce votes by all available user's voting power.
Status
NEW
Client's comments
comment here
INFORMATIONAL
5. Ill-conceived logic of access rights in LA.sol
Description
Although there is a separate method for set the minter, it can be set only once, as there is a require(minter == address(0), "Already set");. Thus, due to the fact the admin account is not used anywhere else, it seems logical to set minter value in the constructor.
Links to the code:
Recommendation
Refactor access model, or clarify it documentation if it required by design.
Status
NEW
Client's comments
comment here
6. Unnecessary calculations
Description
In LA.sol contract constant INITIAL_RATE is defined by the expression 100638977635782747603833865 / YEAR, which:
- leads to loss of precision in integer division
- leads to extra bytecode (and therefore more gas consumption) when deploying a contract
Links to the code:
Recommendation
It is recommended to calculate absolute vakue of the constant given the division.
Status
NEW
Client's comments
comment here
7. Unused imports
Description
The imported external contracts are not used in code.
Links to the code:
- https://github.com/litrafi/litra-contract/blob/26eb98aa2709707db72c9d9d065eba7f6616486c/contracts/tokenize/WrappedNFT.sol#L4
- https://github.com/litrafi/litra-contract/blob/26eb98aa2709707db72c9d9d065eba7f6616486c/contracts/dao/admin/Stoppable.sol#L4
Recommendation
It is recommended to remove unused imports.
Status
NEW
Client's comments
comment here
8. Addresses in constructor are not checked to zero.
Description
In constructors, when specifying addresses used inside a contract, there is no check for a zero value, which may lead to the need to re-deploy contracts in case of an error.
Links to the code:
- https://github.com/litrafi/litra-contract/blob/26eb98aa2709707db72c9d9d065eba7f6616486c/contracts/tokenize/WrappedNFT.sol#L11
- https://github.com/litrafi/litra-contract/blob/26eb98aa2709707db72c9d9d065eba7f6616486c/contracts/dao/burner/SimpleBurner.sol#L21
Recommendation
It is recommended to add checks for zero.
Status
NEW
Client's comments
comment here
9. Text typos.
Description
There are some typos in code:
Links to the code:
Recommendation
It is recommended to fix the typos.
Status
NEW
Client's comments
comment here
