LMCV part 2 - DAMfinance

Prepared by:

HALBORN

Last Updated 04/26/2024

Date of Engagement by: September 8th, 2022 - September 29th, 2022

Summary

75% of all REPORTED Findings have been addressed

All findings

Critical

High

Medium

Low

Informational

1. Introduction
2. Audit summary
3. Test approach & methodology
4. Scope
5. Risk methodology
6. Scope
7. Assessment summary & findings overview
8. Findings & Tech Details

8.1 User funds may get locked in join contracts
8.2 Users may not be able to unstake or claim rewards when stakedamountlimit is decreased
8.3 Integer underflows in stakingvault module
8.4 Data returned from chainlink is not validated
8.5 Administrators are allowed to burn users ddprime tokens without authorization
8.6 Contracts might lose administrator functionality
8.7 Improper role-based access control
8.8 Missing pause/unpause functionality
8.9 Missing data validation
8.10 Missing zero address checks
8.11 Missing events on changes
8.12 Require message not updated
8.13 Function could be defined as external
8.14 Missing natspec documentation
8.15 Variables could be defined as immutable
8.16 Changing public constant variables to private can save gas

9. Automated Testing

1. INTRODUCTION

DAMfinance engaged Halborn to conduct a security audit on their smart contracts beginning on 2022-09-08 and ending on 2022-09-29. The security assessment was scoped to the smart contracts provided to the Halborn team.

2. AUDIT SUMMARY

The team at Halborn was provided three weeks for the engagement and assigned a full-time security engineer to audit the security of the smart contract. The security engineer is a blockchain and smart-contract security expert with advanced penetration testing, smart-contract hacking, and deep knowledge of multiple blockchain protocols.

The purpose of this audit is to:

Ensure that smart contract functions operate as intended
Identify potential security issues with the smart contracts

In summary, Halborn identified some security risks that were mostly addressed by the DAMfinance team.

3. TEST APPROACH & METHODOLOGY

Halborn performed a combination of manual and automated security testing to balance efficiency, timeliness, practicality, and accuracy in regard to the scope of this audit. While manual testing is recommended to uncover flaws in logic, process, and implementation; automated testing techniques help enhance coverage of the bridge code and can quickly identify items that do not follow security best practices. The following phases and associated tools were used throughout the term of the audit:

Research into architecture and purpose
Smart contract manual code review and walk-through
Graphing out functionality and contract logic/connectivity/functions (solgraph)
Manual assessment of use and safety for the critical Solidity variables and functions in scope to identify any arithmetic related vulnerability classes
Manual testing by custom scripts
Static Analysis of security for scoped contract, and imported functions. (Slither)
Local deployment (Hardhat, Remix IDE, Brownie)

4. SCOPE

IN-SCOPE: The security assessment was scoped to the following smart contracts:

new LMCV contracts:

lmcv/AuctionHouse.sol
lmcv/ChainlinkClient.sol
lmcv/Liquidator.sol
lmcv/OSM.sol
lmcv/PriceUpdater.sol
lmcv/PSM.sol
lmcv/RatesUpdater.sol

Staking:

staking/RewardJoin.sol
staking/StakeJoin.sol
staking/StakingVault.sol
staking/ddPrime.sol
staking/ddPrimeJoin.sol

Commit ID: f5861988508a5bb291a3a7f2863693cd9762dee7 Pull request: 25

Remediation plan:

Pull Request: 30 Branch: secondRoundAuditFixes Commit ID 9798fb6f03aab96d8702116e6bef394b2e501d59

OUT-OF-SCOPE: Other smart contracts in the repository, external libraries and economical attacks.

5. RISK METHODOLOGY

Vulnerabilities or issues observed by Halborn are ranked based on the risk assessment methodology by measuring the LIKELIHOOD of a security incident and the IMPACT should an incident occur. This framework works for communicating the characteristics and impacts of technology vulnerabilities. The quantitative model ensures repeatable and accurate measurement while enabling users to see the underlying vulnerability characteristics that were used to generate the Risk scores. For every vulnerability, a risk level will be calculated on a scale of 5 to 1 with 5 being the highest likelihood or impact.

RISK SCALE - LIKELIHOOD

5 - Almost certain an incident will occur.
4 - High probability of an incident occurring.
3 - Potential of a security incident in the long term.
2 - Low probability of an incident occurring.
1 - Very unlikely issue will cause an incident.

RISK SCALE - IMPACT

5 - May cause devastating and unrecoverable impact or loss.
4 - May cause a significant level of impact or loss.
3 - May cause a partial impact or loss to many.
2 - May cause temporary impact or loss.
1 - May cause minimal or un-noticeable impact.

The risk level is then calculated using a sum of these two values, creating a value of 10 to 1 with 10 being the highest level of security risk.

Critical

High

Medium

Low

Informational

10 - CRITICAL
9 - 8 - HIGH
7 - 6 - MEDIUM
5 - 4 - LOW
3 - 1 - VERY LOW AND INFORMATIONAL

6. SCOPE

Out-of-Scope: New features/implementations after the remediation commit IDs.

7. Assessment Summary & Findings Overview

Critical

High

Medium

Low

Informational

Impact x Likelihood

Likelihood

Impact

HAL-02

HAL-05

HAL-01

HAL-04

HAL-06

HAL-07

HAL-08

HAL-03

HAL-10

HAL-09

HAL-11

HAL-12

HAL-13

HAL-14

HAL-15

HAL-16

Security analysis	Risk level	Remediation Date
USER FUNDS MAY GET LOCKED IN JOIN CONTRACTS	High	Solved - 10/10/2022
USERS MAY NOT BE ABLE TO UNSTAKE OR CLAIM REWARDS WHEN stakedAmountLimit IS DECREASED	Medium	Solved - 10/11/2022
INTEGER UNDERFLOWS IN STAKINGVAULT MODULE	Medium	Solved - 10/11/2022
DATA RETURNED FROM CHAINLINK IS NOT VALIDATED	Medium	Solved - 10/10/2022
ADMINISTRATORS ARE ALLOWED TO BURN USERS DDPRIME TOKENS WITHOUT AUTHORIZATION	Medium	Future Release
CONTRACTS MIGHT LOSE ADMINISTRATOR FUNCTIONALITY	Low	Solved - 10/10/2022
IMPROPER ROLE-BASED ACCESS CONTROL	Low	Risk Accepted
MISSING PAUSE/UNPAUSE FUNCTIONALITY	Low	Solved - 10/11/2022
MISSING DATA VALIDATION	Low	Partially Solved - 10/11/2022
MISSING ZERO ADDRESS CHECKS	Informational	-
MISSING EVENTS ON CHANGES	Informational	Solved - 10/11/2022
REQUIRE MESSAGE NOT UPDATED	Informational	-
FUNCTION COULD BE DEFINED AS EXTERNAL	Informational	Solved - 10/11/2022
MISSING NATSPEC DOCUMENTATION	Informational	Future Release
VARIABLES COULD BE DEFINED AS IMMUTABLE	Informational	Solved - 10/11/2022
CHANGING PUBLIC CONSTANT VARIABLES TO PRIVATE CAN SAVE GAS	Informational	Solved - 10/11/2022

8. Findings & Tech Details

8.1 USER FUNDS MAY GET LOCKED IN JOIN CONTRACTS

// High

Description

The RewardJoin and StakeJoin contracts have possibility to be stopped/disabled by the administrator using cage function.

When the administrator calls the cage function, user funds will get locked as the users will not be able to call the exit function successfully and get the staked and rewards tokens.

The contracts also do not implement a possibility to start the contract again (setting the live flag to 1), so all funds will get locked inside the contract.

Code Location

cage function:

contracts/staking/RewardJoin.sol

function cage() external auth {
    live = 0;
    emit Cage();
}

staking/RewardJoin.sol

contracts/staking/RewardJoin.sol

function exit(address usr, uint256 wad) external {
    require(live == 1, "CollateralJoin/not-live");
    stakingVault.pullRewards(collateralName, msg.sender, wad);
    require(collateralContract.transfer(usr, wad), "RewardJoin/failed-transfer");
    emit Exit(usr, wad);
}

staking/StakeJoin.sol

contracts/staking/StakeJoin.sol

function exit(address usr, uint256 wad) external {
    require(live == 1, "CollateralJoin/not-live");
    stakingVault.pullStakingToken(msg.sender, wad);
    require(collateralContract.transfer(usr, wad), "CollateralJoin/failed-transfer");
    emit Exit(usr, wad);
}

Example Hardhat test cases:

it("HAL-01 User staked tokens are locked (StakeJoin)", async function () {
    // User stakes tokens
    let userStakeJoin = stakeJoin.connect(addr1);
    await userStakeJoin.join(addr1.address, fwad("1000"));

    // Contract is stopped
    await stakeJoin.cage();

    // User can't exit, collateral is locked in the contract
    await expect(userStakeJoin.exit(addr1.address, fwad("500")))
        .to.be.revertedWith("CollateralJoin/not-live");
});

it("HAL-01 User rewards are locked (RewardsJoin)", async function () {
    // User stakes tokens
    let userStakeJoin = stakeJoin.connect(addr1);
    await userStakeJoin.join(addr1.address, fwad("1000"));

    // User stakes tokens in the vault
    userSV = stakingVault.connect(addr1);
    await userSV.stake(fwad("800"), addr1.address);

    // Rewards are added
    await fooJoin.join(fwad("1000"));

    // User stakes 0 to claim rewards
    await userSV.stake("0", addr1.address);

    // Contract is stopped
    await fooJoin.cage();

    // User tries to exit rewards join contract
    let userFooJoin1 = fooJoin.connect(addr1);
    await expect(userFooJoin1.exit(addr1.address, "1000000000000000000000"))
        .to.be.revertedWith("CollateralJoin/not-live");
});

Score

Impact: 5
Likelihood: 3

Recommendation

SOLVED: The cage function in the RewardJoin and StakeJoin contracts was updated, adding a possibility to make the contract live again.

Reference: RewardJoin.sol and StakeJoin.sol

8.2 USERS MAY NOT BE ABLE TO UNSTAKE OR CLAIM REWARDS WHEN stakedAmountLimit IS DECREASED

// Medium

Description

The StakingVault contract defines a maximum amount allowed to stake in the stakedAmountLimit variable. An administrator can modify this value. However, the setStakedAmountLimit function does not perform any validation on its parameter.

It is possible to decrease stakedAmountLimit to a value lower than the currently staked amount. In such a situation, some users may not be able to unstake or claim their rewards because of the require statement in the stake function, enforcing that stakedAmount must be lower or equal to the limit.

Example scenario:

stakedAmountLimit is set to 5000
User 1 stakes 4000 tokens
User 2 stakes 1000 tokens
Administrator decreases stakedAmountLimit to 3000
User 2 can't unstake/claim rewards as transaction gets reverted due to require statement

In the above scenario, User 2 won't be able to unstake or claim the rewards until User 1 decreases his staked amount.

Code Location

setStakedAmountLimit setter:

contracts/staking/StakingVault.sol

function setStakedAmountLimit(uint256 wad) external auth {
    stakedAmountLimit = wad;
    emit StakedAmountLimit(wad);
}

Unstaking may not be possible, because of the require statement in stake function, enforcing that current stakedAmount + new staked amount is lower than the limit:

contracts/staking/StakingVault.sol

function stake(int256 wad, address user) external stakeAlive { // [wad]
    require(approval(user, msg.sender), "StakingVault/Owner must consent");
    require(getOwnedDDPrime(user) >= lockedStakeable[user] * stakedMintRatio, "StakingVault/Need to own ddPRIME to cover locked amount");

    //1. Add locked tokens
    uint256 prevStakedAmount    = lockedStakeable[user]; //[wad]
    unlockedStakeable[user]     = _sub(unlockedStakeable[user], wad);
    lockedStakeable[user]       = _add(lockedStakeable[user], wad);
    stakedAmount                = _add(stakedAmount, wad);
    require(stakedAmount <= stakedAmountLimit, "StakingVault/Cannot be over staked token limit");
[...]

Hardhat test case:

it("HAL-02 User may not be able to unstake/claim rewards when stakedAmountLimit is decreased", async function () {

    //User 1 and 2 add 10000 stakeable coin with join contract
    await userStakeJoin.join(addr1.address, fwad("10000"));
    await userStakeJoin2.join(addr2.address, fwad("10000"));

    // User1 stakes 4000 of tokens (stakedAmount< limit)
    await userSV.stake(fwad("4000"), addr1.address);

    // assert balances
    expect(await stakingVault.lockedStakeable(addr1.address)).to.equal(fwad("4000"));
    expect(await stakingVault.unlockedStakeable(addr1.address)).to.equal(fwad("6000"));
    expect(await stakingVault.ddPrime(addr1.address)).to.equal(frad("4000"));

    // User2 stakes 1000 of tokens  (stakedAmount == limit)
    await userSV2.stake(fwad("1000"), addr2.address);

    // assert balances
    expect(await stakingVault.lockedStakeable(addr2.address)).to.equal(fwad("1000"));
    expect(await stakingVault.unlockedStakeable(addr2.address)).to.equal(fwad("9000"));
    expect(await stakingVault.ddPrime(addr2.address)).to.equal(frad("1000"));

    // Foo Rewards get added
    await fooJoin.join(fwad("50"));

    // limit is changed to 3000
    await stakingVault.setStakedAmountLimit(fwad("3000"));

    // user 2 stakes 0 to claim rewards - transaction fails
    await expect(userSV2.stake(fwad("0"), addr2.address))
        .to.be.revertedWith("StakingVault/Cannot be over staked token limit");

    // user 2 tries to unstake -1000 tokens to claim rewards - transaction fails
    await expect(userSV2.stake(fwad("-1000"), addr2.address))
        .to.be.revertedWith("StakingVault/Cannot be over staked token limit");

    // User 1 stakes 0 to claim rewards - transaction fails, without reducing staked tokens
    await expect(userSV.stake(fwad("0"), addr1.address))
        .to.be.revertedWith("StakingVault/Cannot be over staked token limit");

    // User 1 reduces staked tokens to claim rewards
    await userSV.stake(fwad("-2000"), addr1.address);

    // assert reward balances
    expect(await stakingVault.rewardDebt(addr1.address, fooBytes)).to.equal(fwad("20"));
    expect(await stakingVault.rewardDebt(addr2.address, fooBytes)).to.equal(0);
    expect(await stakingVault.withdrawableRewards(addr1.address, fooBytes)).to.equal(fwad("40"));
    expect(await stakingVault.withdrawableRewards(addr2.address, fooBytes)).to.equal(0);

    // Now user 2 is able to claim rewards/unstake
    await userSV2.stake(fwad("0"), addr2.address);
    expect(await stakingVault.rewardDebt(addr2.address, fooBytes)).to.equal(fwad("10"));
    expect(await stakingVault.withdrawableRewards(addr2.address, fooBytes)).to.equal(fwad("10"));
});

Score

Impact: 5
Likelihood: 2

Recommendation

SOLVED: The require statement in the stake function was modified to always allow a negative amount (withdraw request) - in this way, the user will always be able to withdraw the amount staked at 0. When the user tries to withdraw more than staked, the transaction is reverted in the _add function.

Reference: StakingVault.sol

8.3 INTEGER UNDERFLOWS IN STAKINGVAULT MODULE

// Medium

Description

There are multiple instances in the StakingVault contract when subtracting balances is done without checks. Such behavior may cause the transaction to fail due to arithmetic errors (integer underflow), for example, when the user balance is lower than the requested withdrawal.

Code Location

The pullStakingToken function is not verifying balance before subtraction in staking/StakingVault.sol:

contracts/staking/StakingVault.sol

function pullStakingToken(address user, uint256 wad) external auth {
    unlockedStakeable[user] -= wad;
    emit PullStakingToken(user, wad);
}

The pullRewards function is not checking if user has enough tokens to move in staking/StakingVault.sol:

staking/StakingVault.sol

function pullRewards(bytes32 rewardToken, address usr, uint256 wad) external auth {
    RewardTokenData storage tokenData = RewardData[rewardToken];
    withdrawableRewards[usr][rewardToken]   -= wad;
    tokenData.totalRewardAmount             -= wad;
    emit PullRewards(rewardToken, usr, wad);
}

The liquidationWithdraw does not check if liquidator has enough ddPrime in staking/StakingVault.sol:

staking/StakingVault.sol

function liquidationWithdraw(address liquidator, address liquidated, uint256 rad) external {
    require(approval(liquidator, msg.sender), "StakingVault/Owner must consent");

    //1. Check that liquidated does not own ddPrime they claim to
    require(getOwnedDDPrime(liquidated) <= lockedStakeable[liquidated] * stakedMintRatio - rad, "StakingVault/Account must not have ownership of tokens");
    uint256 liquidatedAmount         = rad / stakedMintRatio; // rad / ray = wad
    uint256 prevStakedAmount         = lockedStakeable[liquidated]; //[wad]

    //2. Take ddPrime from liquidator's account to repay
    ddPrime[liquidator]             -= rad;
    totalDDPrime                    -= rad;
[...]

Example Hardhat test cases:

it("HAL-03 Integer underflow - withdraw more than staked", async function () {
    // User tries to exit from stake join contract without staking anything before
    let userStakeJoin = stakeJoin.connect(addr1);
    await expect(userStakeJoin.exit(addr1.address, fwad("500")))
        .to.be.revertedWith("Arithmetic operation underflowed or overflowed outside of an unchecked block");
});

it("HAL-03 Integer underflow - withdraw more than withdrawable (RewardJoin)", async function () {
    // User joins stake
    let userStakeJoin = stakeJoin.connect(addr1);
    await userStakeJoin.join(addr1.address, fwad("1000"));

    // User stakes tokens in the vault
    userSV = stakingVault.connect(addr1);
    await userSV.stake(fwad("800"), addr1.address);

    // Rewards are added
    await fooJoin.join(fwad("1000"));

    // User stakes 0 to claim rewards
    await userSV.stake("0", addr1.address);
    expect(await stakingVault.withdrawableRewards(addr1.address, fooBytes)).to.equal(fwad("1000"));

    // User tries to exit reward join contract with more tokens than withdrawable
    let userFooJoin1 = fooJoin.connect(addr1);
    await expect(userFooJoin1.exit(addr1.address, fwad("10000")))
        .to.be.revertedWith("Arithmetic operation underflowed or overflowed outside of an unchecked block");
});

it("HAL-03 Integer underflow - Liquidator doesn't hold enough ddPrime", async function () {
    // Sanity checks
    expect(await lmcv.lockedCollateral(addr4.address, ddPrimeBytes)).to.equal(fwad("1000"));
    expect(await lmcv.lockedCollateral(addr4.address, blorpBytes)).to.equal(fwad("1000"));
    expect(await ddPrime.balanceOf(addr4.address)).to.equal(0);
    expect(await stakingVault.ddPrime(addr4.address)).to.equal(0);
    expect(await stakingVault.getOwnedDDPrime(addr4.address)).to.equal(frad("1000"));
    expect(await lmcv.normalizedDebt(addr4.address)).to.equal(fwad("100"));

    // User 4 stakes 1000 to claim rewards
    await userStakeJoin4.join(addr4.address, fwad("1000"));
    await userSV4.stake(fwad("1000"), addr4.address);

    // Assert rewards
    expect(await stakingVault.withdrawableRewards(addr4.address, fooBytes)).to.equal("0");
    expect(await stakingVault.rewardDebt(addr4.address, fooBytes)).to.equal(fwad("20"));

    // Foo Rewards get added
    await fooJoin.join(fwad("20"));

    // User 4 stakes 0 to claim rewards
    await userSV4.stake(fwad("0"), addr4.address);

    // User 1 gets liquidated, ddPrime is transferred to user 3 (bypasses auction functionality)
    await lmcv.seize([ddPrimeBytes], [fwad("1000")], fwad("100"), addr4.address, addr3.address, owner.address);

    // User 3 calls liquidationWithdraw not having enough ddPrime
    await expect(userSV3.liquidationWithdraw(addr3.address, addr4.address, frad("1000")))
        .to.be.revertedWith("Arithmetic operation underflowed or overflowed outside of an unchecked block");
});

Score

Impact: 3
Likelihood: 3

Recommendation

SOLVED: Additional require statements were added, to ensure underflow does not occur:

StakingVault.sol: #216, 238, 280

8.4 DATA RETURNED FROM CHAINLINK IS NOT VALIDATED

// Medium

Description

The getLatestPrice function in the contract ChainlinkClient.sol fetches the asset price from a Chainlink aggregator using the latestRoundData function. However, there are no checks on roundID nor timeStamp, which may result in stale prices.

If there is a problem with Chainlink starting a new round and finding consensus on the new value for the oracle (e.g., Chainlink nodes abandon the oracle, chain congestion, vulnerability/attacks on the Chainlink system), consumers of this contract may continue using outdated stale data (if oracles are unable to submit no new round is started).

Code Location

contracts/lmcv/ChainlinkClient.sol

function getLatestPrice() public view returns (int256) {
    (
        /*uint80 roundID*/,
        int256 price,
        /*uint startedAt*/,
        /*uint timeStamp*/,
        /*uint80 answeredInRound*/
    ) = priceFeed.latestRoundData();
    return price;
}

Score

Impact: 4
Likelihood: 2

Recommendation

SOLVED: Validation of data returned from Chainlink was added.

Reference: ChainlinkClient.sol

8.5 ADMINISTRATORS ARE ALLOWED TO BURN USERS DDPRIME TOKENS WITHOUT AUTHORIZATION

// Medium

Description

An administrator of the ddPrime token can burn users' tokens. Such action might break the StakingVault contract, as it keeps its copy of ddPrime balances.

Code Location

ddPrime.sol

staking/ddPrime.sol

function burn(address from, uint256 value) external {
    uint256 balance = balanceOf[from];
    require(balance >= value, "ddPrime/insufficient-balance");

    if (from != msg.sender && admins[msg.sender] != 1) {
        uint256 allowed = allowance[from][msg.sender];
        if (allowed != type(uint256).max) {
            require(allowed >= value, "ddPrime/insufficient-allowance");

            unchecked {
                allowance[from][msg.sender] = allowed - value;
            }
        }
    }
[...]

Hardhat test scenario:

it("HAL-05 Administrator burns users ddPrime", async function () {

    //User 1 joins and stakes 1000 tokens
    await userStakeJoin.join(addr4.address, fwad("10000"));
    await userSV4.stake(fwad("1000"), addr4.address);

    // Approve and exit with ddPrime
    await userSV4.approve(ddPrimeJoin.address);
    userDDPrimeJoin = ddPrimeJoin.connect(addr4);
    await userDDPrimeJoin.exit(addr4.address, fwad("1000"));

    // Assert balance
    expect(await stakingVault.ddPrime(addr4.address)).to.equal(0);
    expect(await ddPrime.balanceOf(addr4.address)).to.equal(fwad("1000"));

    // Administrator burns ddPrime
    await ddPrime.burn(addr4.address, fwad("500"));

    // Assert balance
    expect(await ddPrime.balanceOf(addr4.address)).to.equal(fwad("500"));
});

Score

Impact: 5
Likelihood: 2

Recommendation

PENDING: The DAMfinance team stated that they plan to implement the recommended fix with the governance module in the future.

8.6 CONTRACTS MIGHT LOSE ADMINISTRATOR FUNCTIONALITY

// Low

Description

The deny function is not checking if there are any other active wards before setting wards[usr] = 0. The contract will lose administrator functionality when the only ward user calls this function.

Code Location

OSM.sol, #32

RatesUpdater.sol, #30

Liquidator.sol, #68

AuctionHouse.sol, #26

staking/ddPrime.sol, #68

staking/RewardJoin.sol, #34

staking/StakeJoin.sol, #32

staking/StakingVault.sol, #102

Score

Impact: 3
Likelihood: 1

Recommendation

SOLVED: The ArchAdmin variable was added to the contract. The address assigned to this field cannot be removed from the wards/admins mapping via the administrate or deny functions, ensuring that there is at least one administrator on the contract. To update this address, a new ArchAdmin must be set; then, the address can be removed from the admin mapping.

8.7 IMPROPER ROLE-BASED ACCESS CONTROL

// Low

Description

The smart contracts in scope do not implement granular access control. All the privileged functionality is assigned to one role. This could lead to severe consequences if, for example, such an account gets compromised or a malicious administrator decides to take over the platform.

Score

Impact: 3
Likelihood: 2

Recommendation

RISK ACCEPTED: The DAM finance team accepted the risk of this finding. In this role-based admin structure, only smart contracts would have access to specific roles, and a person-controlled owner address would have the ability to set all of these roles. Since a smart contract can only call functions it has interfaces for and admin access to in this setup, and an owner-level admin hack would have the ability to set itself as any other level admin, this does not seem like a useful check.

8.8 MISSING PAUSE/UNPAUSE FUNCTIONALITY

// Low

Description

In case a hack occurs, or an exploit is discovered, the team should be able to pause functionality until the necessary changes are made to the system.

To use a THORchain example again, the team behind the THORchain noticed an attack was going to occur well before the system transferred funds to the hacker. However, they were unable to shut the system down fast enough (According to the incident report).

Following contracts: Liquidator, AuctionHouse, RewardsJoin, and StakeJoin implement a cage function which the administrator may use to stop the contract. However, there is no possibility of putting the contract into a live state again.

The StakingVault contract has a stakeLive flag, which the stakeAlive modifier uses to allow access to the stake function. During an audit, it was discovered that the contract couldn't be paused/stopped. The stakeLive flag is set to 1 in the constructor, and there is no method to change it later.

Code Location

lmcv/Liquidator.sol, #125

lmcv/AuctionHouse.sol, #92

stake/RewardsJoin.sol, #71

stake/StakeJoin.sol, #69

stake/StakingVault.sol, #93

Score

Impact: 3
Likelihood: 2

Recommendation

SOLVED: The cage function was modified, and the setStakeAlive function was added to the contracts. Now, contracts can be stopped/resumed in case of an attack.

8.9 MISSING DATA VALIDATION

// Low

Description

Configuration values in the contracts are not validated; it is possible to set large and/or invalid values, causing contracts to fail.

Code Location

OSM.sol, #99

The pokeTimeout has no maximum value and can be set to a very large value, making calling poke impossible in a reasonable time.

RatesUpdater.sol, #103

The value of stabilityRate is not validated. When stability rate is set to an invalid value (not per the second rate), for example, percentage: 1.05, calls to accrueInterest will revert during rate calculation in the _rpow function.

AuctionHouse.sol, #96, 100, 104, 108

Bid/auction expirity do not have maximum value. The minimumBidIncrease and minimumBidDecrease do not have max/min values. Setting minimumBidDecrease >= 1.0 will cause users to be able to bid any value.

Liquidator.sol, #129, 133

The lotSize variable can be set to 0, causing the liquidate function to fail each time because of the require statement:

lmcv/Liquidator.sol

function liquidate(address user) external {
    require(live == 1, "Liquidator/Not live");
    require(liquidationPenalty != 0 && lotSize != 0, "Liquidator/Not set up");
    require(!lmcv.isWithinCreditLimit(user, lmcv.AccumulatedRate()), "Liquidator/Vault within credit limit");
[...]

Score

Impact: 2
Likelihood: 2

Recommendation

PARTIALLY SOLVED: The lotSize validation was added in the Liquidator.sol contract. However, the \client team decided to leave other variables without validation.

8.10 MISSING ZERO ADDRESS CHECKS

// Informational

Description

Finding description placeholder

Score

Impact:
Likelihood:

8.11 MISSING EVENTS ON CHANGES

// Informational

Description

Functions performing important changes on tchangePokeTimeout,start,stop,void,updateSource,cage,changeStabilityRate` are not emitting events to facilitate monitoring of the protocol.

Code Location

lmcv/RatesUpdater.sol, #102

lmcv/PriceUpdater.sol, #78, 84

lmcv/OSM.sol, #86, 87, 92, 101, 112

lmcv/AuctionHouse.sol, #92, 96, 100, 104, 108

lmcv/Liquidator.sol, #125, 129, 133, 139, 149

staking/StakingVault.sol, #101, 106, 110

Score

Impact: 1
Likelihood: 1

Recommendation

SOLVED: New events were added, except approval events for StakingVault:

RatesUpdater.sol: #132
PriceUpdater.sol: #103, 111
OSM.sol: #110, 111, 119, 128, 142
AuctionHouse.sol: #107, 112, 117, 122, 127
Liquidator.sol: #146, 151, 157, 171, 179
StakingVault.sol, #169

lmcv/PriceUpdater.sol

LMCVLike    public lmcv;  // CDP Engine

RatesUpdater:

lmcv/RatesUpdater.sol

LMCVLike    public lmcv;            // LMCV contract

Score

Impact: 1
Likelihood: 1

Recommendation

SOLVED: Variable definitions were updated:

8.16 CHANGING PUBLIC CONSTANT VARIABLES TO PRIVATE CAN SAVE GAS

// Informational

Description

Some constants are public and have a getter function. It is unlikely for these values to be read from the outside. Therefore, it is not necessary to make them public.

Code Location

LMCVProxy

lmcv/LMCVProxy.sol

uint256 constant RAY = 10 ** 27;

OSM

lmcv/OSM.sol

uint256     constant    ONE_HOUR        = 3600;

StakingVault

staking/StakingVault.sol

uint256 constant RAY = 10 ** 27;

Score

Impact: 1
Likelihood: 1

Recommendation

SOLVED: Constant definitions were updated:

9. Automated Testing

STATIC ANALYSIS REPORT

Description

Halborn used automated testing techniques to enhance the coverage of certain areas of the smart contract in scope. Among the tools used was Slither, a Solidity static analysis framework. After Halborn verified the smart contract in the repository and was able to compile it correctly into its ABI and binary format, Slither was run against the contract. This tool can statically verify mathematical relationships between Solidity variables to detect invalid or inconsistent usage of the contracts' APIs across the entire code-base.

Slither results

OSM.prev(uint256) (contracts/lmcv/OSM.sol#176-179) uses a weak PRNG: "ts - (ts % pokeTimeout) (contracts/lmcv/OSM.sol#178)" 
RatesUpdater._rpow(uint256,uint256,uint256) (contracts/lmcv/RatesUpdater.sol#64-86) uses a weak PRNG: "switch_expr_2184_53_20__rpow_asm_0 = n % 2 (contracts/lmcv/RatesUpdater.sol#68)" 
RatesUpdater._rpow(uint256,uint256,uint256) (contracts/lmcv/RatesUpdater.sol#64-86) uses a weak PRNG: "n % 2 (contracts/lmcv/RatesUpdater.sol#76-82)" 
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#weak-PRNG

StakingVaultLike is re-used:
        - StakingVaultLike (contracts/staking/RewardJoin.sol#10-14)
        - StakingVaultLike (contracts/staking/StakeJoin.sol#10-13)
        - StakingVaultLike (contracts/staking/ddPrimeJoin.sol#12-14)
LMCVLike is re-used:
        - LMCVLike (contracts/lmcv/AuctionHouse.sol#7-10)
        - LMCVLike (contracts/lmcv/Liquidator.sol#7-34)
        - LMCVLike (contracts/lmcv/PSM.sol#12-29)
        - LMCVLike (contracts/lmcv/PriceUpdater.sol#18-20)
        - LMCVLike (contracts/lmcv/RatesUpdater.sol#7-10)
        - LMCVLike (contracts/staking/StakingVault.sol#11-14)
dPrimeJoinLike is re-used:
        - dPrimeJoinLike (contracts/lmcv/PSM.sol#6-10)
CollateralLike is re-used:
        - CollateralLike (contracts/staking/RewardJoin.sol#5-8)
        - CollateralLike (contracts/staking/StakeJoin.sol#5-8)
CollateralJoinLike is re-used:
        - CollateralJoinLike (contracts/lmcv/PSM.sol#38-44)
dPrimeLike is re-used:
        - dPrimeLike (contracts/lmcv/PSM.sol#31-36)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#name-reused

Liquidator.liquidate(address) (contracts/lmcv/Liquidator.sol#178-253) performs a multiplication on the result of a division:
        -debtHaircut = min(normalizedDebt,lotSize * RAY / stabilityRate * RAY / liquidationPenalty) (contracts/lmcv/Liquidator.sol#192)
RatesUpdater._rpow(uint256,uint256,uint256) (contracts/lmcv/RatesUpdater.sol#64-86) performs a multiplication on the result of a division:
        -x = xxRound__rpow_asm_0 / b (contracts/lmcv/RatesUpdater.sol#75)
        -zx__rpow_asm_0 = z * x (contracts/lmcv/RatesUpdater.sol#77)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#divide-before-multiply

RatesUpdater._rpow(uint256,uint256,uint256) (contracts/lmcv/RatesUpdater.sol#64-86) uses a dangerous strict equality:
        - switch_expr_2113_41_20__rpow_asm_0 == 0 (contracts/lmcv/RatesUpdater.sol#66)
RatesUpdater._rpow(uint256,uint256,uint256) (contracts/lmcv/RatesUpdater.sol#64-86) uses a dangerous strict equality:
        - switch_expr_2184_53_20__rpow_asm_0 == 0 (contracts/lmcv/RatesUpdater.sol#68)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#dangerous-strict-equalities

Reentrancy in AuctionHouse.converge(uint256,uint256) (contracts/lmcv/AuctionHouse.sol#207-236):
        External calls:
        - lmcv.moveDPrime(msg.sender,auctions[id].currentWinner,auctions[id].debtBid) (contracts/lmcv/AuctionHouse.sol#233)
        State variables written after the call(s):
        - auctions[id].currentWinner = msg.sender (contracts/lmcv/AuctionHouse.sol#234)
Reentrancy in OSM.poke() (contracts/lmcv/OSM.sol#133-142):
        External calls:
        - (wut,ok) = OracleLike(oracleAddress).peek() (contracts/lmcv/OSM.sol#135)
        State variables written after the call(s):
        - zzz = prev(block.timestamp) (contracts/lmcv/OSM.sol#139)
Reentrancy in AuctionHouse.raise(uint256,uint256) (contracts/lmcv/AuctionHouse.sol#169-197):
        External calls:
        - lmcv.moveDPrime(msg.sender,auctions[id].currentWinner,auctions[id].debtBid) (contracts/lmcv/AuctionHouse.sol#190)
        State variables written after the call(s):
        - auctions[id].currentWinner = msg.sender (contracts/lmcv/AuctionHouse.sol#191)
Reentrancy in AuctionHouse.raise(uint256,uint256) (contracts/lmcv/AuctionHouse.sol#169-197):
        External calls:
        - lmcv.moveDPrime(msg.sender,auctions[id].currentWinner,auctions[id].debtBid) (contracts/lmcv/AuctionHouse.sol#190)
        - lmcv.moveDPrime(msg.sender,auctions[id].treasury,bid - auctions[id].debtBid) (contracts/lmcv/AuctionHouse.sol#193)
        State variables written after the call(s):
        - auctions[id].debtBid = bid (contracts/lmcv/AuctionHouse.sol#195)
        - auctions[id].bidExpiry = uint256(block.timestamp) + bidExpiry (contracts/lmcv/AuctionHouse.sol#196)
Reentrancy in Liquidator.setAuctionHouse(address) (contracts/lmcv/Liquidator.sol#149-153):
        External calls:
        - lmcv.disapprove(address(auctionHouse)) (contracts/lmcv/Liquidator.sol#150)
        State variables written after the call(s):
        - auctionHouse = AuctionHouseLike(addr) (contracts/lmcv/Liquidator.sol#151)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-1

OSM.changeOracleAddress(address)._oracleAddress (contracts/lmcv/OSM.sol#93) lacks a zero-check on :
                - oracleAddress = _oracleAddress (contracts/lmcv/OSM.sol#95)
PSM.constructor(address,address,address).treasury_ (contracts/lmcv/PSM.sol#111) lacks a zero-check on :
                - treasury = treasury_ (contracts/lmcv/PSM.sol#119)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#missing-zero-address-validation

AuctionHouse.start(address,address,uint256,bytes32[],uint256[],uint256,uint256) (contracts/lmcv/AuctionHouse.sol#131-159) has external calls inside a loop: lmcv.moveCollateral(lotList[i],msg.
sender,address(this),lotValues[i]) (contracts/lmcv/AuctionHouse.sol#157)
AuctionHouse.converge(uint256,uint256) (contracts/lmcv/AuctionHouse.sol#207-236) has external calls inside a loop: lmcv.moveCollateral(auctions[id].lotList[i],address(this),auctions[id].liqui
dated,portionToReturn) (contracts/lmcv/AuctionHouse.sol#224)
AuctionHouse.end(uint256) (contracts/lmcv/AuctionHouse.sol#243-256) has external calls inside a loop: lmcv.moveCollateral(auctions[id].lotList[i],address(this),auctions[id].currentWinner,rema
iningCollateral) (contracts/lmcv/AuctionHouse.sol#253)
Liquidator.liquidate(address) (contracts/lmcv/Liquidator.sol#178-253) has external calls inside a loop: amount = lmcv.lockedCollateral(user,collateralList[i]) (contracts/lmcv/Liquidator.sol#2
09)
Liquidator.liquidate(address) (contracts/lmcv/Liquidator.sol#178-253) has external calls inside a loop: (spotPrice) = lmcv.CollateralData(collateralList[i]) (contracts/lmcv/Liquidator.sol#214
)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation/#calls-inside-a-loop

Reentrancy in OSM.poke() (contracts/lmcv/OSM.sol#133-142):
        External calls:
        - (wut,ok) = OracleLike(oracleAddress).peek() (contracts/lmcv/OSM.sol#135)
        State variables written after the call(s):
        - cur = nxt (contracts/lmcv/OSM.sol#137)
        - nxt = Data(wut,1) (contracts/lmcv/OSM.sol#138)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-2

Reentrancy in RewardJoin.exit(address,uint256) (contracts/staking/RewardJoin.sol#99-104):
        External calls:
        - stakingVault.pullRewards(collateralName,msg.sender,wad) (contracts/staking/RewardJoin.sol#101)
        - require(bool,string)(collateralContract.transfer(usr,wad),RewardJoin/failed-transfer) (contracts/staking/RewardJoin.sol#102)
        Event emitted after the call(s):
        - Exit(usr,wad) (contracts/staking/RewardJoin.sol#103)
Reentrancy in StakeJoin.exit(address,uint256) (contracts/staking/StakeJoin.sol#97-102):
        External calls:
        - stakingVault.pullStakingToken(msg.sender,wad) (contracts/staking/StakeJoin.sol#99)
        - require(bool,string)(collateralContract.transfer(usr,wad),CollateralJoin/failed-transfer) (contracts/staking/StakeJoin.sol#100)
        Event emitted after the call(s):
        - Exit(usr,wad) (contracts/staking/StakeJoin.sol#101)
Reentrancy in ddPrimeJoin.exit(address,uint256) (contracts/staking/ddPrimeJoin.sol#56-60):
        External calls:
        - stakingVault.moveDDPrime(msg.sender,address(this),RAY * wad) (contracts/staking/ddPrimeJoin.sol#57)
        - ddPrime.mint(usr,wad) (contracts/staking/ddPrimeJoin.sol#58)
        Event emitted after the call(s):
        - Exit(usr,wad) (contracts/staking/ddPrimeJoin.sol#59)
Reentrancy in RewardJoin.join(uint256) (contracts/staking/RewardJoin.sol#92-97):
        External calls:
        - require(bool,string)(collateralContract.transferFrom(msg.sender,address(this),wad),RewardJoin/failed-transfer) (contracts/staking/RewardJoin.sol#94)
        - stakingVault.pushRewards(collateralName,wad) (contracts/staking/RewardJoin.sol#95)
        Event emitted after the call(s):
        - Join(wad) (contracts/staking/RewardJoin.sol#96)
Reentrancy in StakeJoin.join(address,uint256) (contracts/staking/StakeJoin.sol#90-95):
        External calls:
        - require(bool,string)(collateralContract.transferFrom(msg.sender,address(this),wad),CollateralJoin/failed-transfer) (contracts/staking/StakeJoin.sol#92)
        - stakingVault.pushStakingToken(usr,wad) (contracts/staking/StakeJoin.sol#93)
        Event emitted after the call(s):
        - Join(usr,wad) (contracts/staking/StakeJoin.sol#94)
Reentrancy in ddPrimeJoin.join(address,uint256) (contracts/staking/ddPrimeJoin.sol#50-54):
        External calls:
        - stakingVault.moveDDPrime(address(this),usr,RAY * wad) (contracts/staking/ddPrimeJoin.sol#51)
        - ddPrime.burn(msg.sender,wad) (contracts/staking/ddPrimeJoin.sol#52)
        Event emitted after the call(s):
        - Join(usr,wad) (contracts/staking/ddPrimeJoin.sol#53)
Reentrancy in Liquidator.liquidate(address) (contracts/lmcv/Liquidator.sol#178-253):
        External calls:
        - lmcv.seize(collateralList,collateralHaircuts,debtHaircut,user,address(this),lmcv.Treasury()) (contracts/lmcv/Liquidator.sol#219)
        - id = auctionHouse.start(user,lmcv.Treasury(),askingAmount,collateralList,collateralHaircuts,0,minimumBid) (contracts/lmcv/Liquidator.sol#242-250)
        Event emitted after the call(s):
        - Liquidated(collateralList,collateralHaircuts,user,debtHaircut,askingAmount,id) (contracts/lmcv/Liquidator.sol#252)
Reentrancy in OSM.poke() (contracts/lmcv/OSM.sol#133-142):
        External calls:
        - (wut,ok) = OracleLike(oracleAddress).peek() (contracts/lmcv/OSM.sol#135)
        Event emitted after the call(s):
        - LogValue(cur.val) (contracts/lmcv/OSM.sol#140)
Reentrancy in PriceUpdater.updatePrice(bytes32) (contracts/lmcv/PriceUpdater.sol#95-101):
        External calls:
        - (val,has) = osms[collateral].peek() (contracts/lmcv/PriceUpdater.sol#97)
        - lmcv.updateSpotPrice(collateral,spot) (contracts/lmcv/PriceUpdater.sol#99)
        Event emitted after the call(s):
        - PriceUpdate(collateral,spot) (contracts/lmcv/PriceUpdater.sol#100)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-3

AuctionHouse.raise(uint256,uint256) (contracts/lmcv/AuctionHouse.sol#169-197) uses timestamp for comparisons
        Dangerous comparisons:
        - require(bool,string)(auctions[id].bidExpiry > block.timestamp || auctions[id].bidExpiry == 0,AuctionHouse/Bid expiry reached) (contracts/lmcv/AuctionHouse.sol#173)
        - require(bool,string)(auctions[id].auctionExpiry > block.timestamp,AuctionHouse/Auction ended) (contracts/lmcv/AuctionHouse.sol#175)
AuctionHouse.converge(uint256,uint256) (contracts/lmcv/AuctionHouse.sol#207-236) uses timestamp for comparisons
        Dangerous comparisons:
        - require(bool,string)(auctions[id].bidExpiry > block.timestamp || auctions[id].bidExpiry == 0,AuctionHouse/Bid expiry reached) (contracts/lmcv/AuctionHouse.sol#211)
        - require(bool,string)(auctions[id].auctionExpiry > block.timestamp,AuctionHouse/Auction ended) (contracts/lmcv/AuctionHouse.sol#213)
AuctionHouse.end(uint256) (contracts/lmcv/AuctionHouse.sol#243-256) uses timestamp for comparisons
        Dangerous comparisons:
        - require(bool,string)(auctions[id].bidExpiry != 0 && (auctions[id].bidExpiry < block.timestamp || auctions[id].auctionExpiry < block.timestamp),AuctionHouse/Auction not finished) (co
ntracts/lmcv/AuctionHouse.sol#245-248)
AuctionHouse.restart(uint256) (contracts/lmcv/AuctionHouse.sol#261-266) uses timestamp for comparisons
        Dangerous comparisons:
        - require(bool,string)(auctions[id].auctionExpiry < block.timestamp,AuctionHouse/Auction not finished) (contracts/lmcv/AuctionHouse.sol#262)
OSM.pass() (contracts/lmcv/OSM.sol#123-125) uses timestamp for comparisons
        Dangerous comparisons:
        - block.timestamp >= zzz + pokeTimeout (contracts/lmcv/OSM.sol#124)
RatesUpdater._rpow(uint256,uint256,uint256) (contracts/lmcv/RatesUpdater.sol#64-86) uses timestamp for comparisons
        Dangerous comparisons:
        - switch_expr_2113_41_20__rpow_asm_0 == 0 (contracts/lmcv/RatesUpdater.sol#66)
        - switch_expr_2184_53_20__rpow_asm_0 == 0 (contracts/lmcv/RatesUpdater.sol#68)
RatesUpdater.accrueInterest() (contracts/lmcv/RatesUpdater.sol#114-121) uses timestamp for comparisons
        Dangerous comparisons:
        - require(bool,string)(block.timestamp >= lastAccrual,RatesUpdater/invalid block.timestamp) (contracts/lmcv/RatesUpdater.sol#115)
ddPrime.permit(address,address,uint256,uint256,uint8,bytes32,bytes32) (contracts/staking/ddPrime.sol#179-203) uses timestamp for comparisons
        Dangerous comparisons:
        - require(bool,string)(block.timestamp <= deadline,ddPrime/permit-expired) (contracts/staking/ddPrime.sol#180)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#block-timestamp

RatesUpdater._rpow(uint256,uint256,uint256) (contracts/lmcv/RatesUpdater.sol#64-86) uses assembly
        - INLINE ASM (contracts/lmcv/RatesUpdater.sol#65-85)
StakingVault.either(bool,bool) (contracts/staking/StakingVault.sol#307-309) uses assembly
        - INLINE ASM (contracts/staking/StakingVault.sol#308)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#assembly-usage

Different versions of Solidity are used:
        - Version used: ['0.8.7', '>=0.4.22<0.9.0', '>=0.8.7', '^0.8.0', '^0.8.7']
        - 0.8.7 (contracts/lmcv/AuctionHouse.sol#3)
        - >=0.8.7 (contracts/lmcv/ChainlinkClient.sol#3)
        - 0.8.7 (contracts/lmcv/Liquidator.sol#3)
        - >=0.8.7 (contracts/lmcv/OSM.sol#18)
        - 0.8.7 (contracts/lmcv/PSM.sol#2)
        - ^0.8.7 (contracts/lmcv/PriceUpdater.sol#16)
        - 0.8.7 (contracts/lmcv/RatesUpdater.sol#3)
        - 0.8.7 (contracts/staking/RewardJoin.sol#3)
        - 0.8.7 (contracts/staking/StakeJoin.sol#3)
        - 0.8.7 (contracts/staking/StakingVault.sol#9)
        - 0.8.7 (contracts/staking/ddPrime.sol#5)
        - 0.8.7 (contracts/staking/ddPrimeJoin.sol#3)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#different-pragma-directives-are-used

AuctionHouse (contracts/lmcv/AuctionHouse.sol#12-278) should inherit from AuctionHouseLike (contracts/lmcv/Liquidator.sol#36-46)
OSM (contracts/lmcv/OSM.sol#24-181) should inherit from OracleLike (contracts/lmcv/OSM.sol#20-22)
StakingVault (contracts/staking/StakingVault.sol#20-335) should inherit from StakingVaultLike (contracts/staking/StakeJoin.sol#10-13)
ddPrime (contracts/staking/ddPrime.sol#7-204) should inherit from dPrimeLike (contracts/lmcv/dPrimeJoin.sol#7-10)
ddPrime (contracts/staking/ddPrime.sol#7-204) should inherit from CollateralLike (contracts/staking/RewardJoin.sol#5-8)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#missing-inheritance

Parameter Liquidator.setMinimumAskingPriceVariables(uint256,uint256,uint256)._collateralFactor (contracts/lmcv/Liquidator.sol#140) is not in mixedCase
Parameter Liquidator.setMinimumAskingPriceVariables(uint256,uint256,uint256)._debtFactor (contracts/lmcv/Liquidator.sol#141) is not in mixedCase
Parameter Liquidator.setMinimumAskingPriceVariables(uint256,uint256,uint256)._debtGrossUpFactor (contracts/lmcv/Liquidator.sol#142) is not in mixedCase
Parameter OSM.changeOracleAddress(address)._oracleAddress (contracts/lmcv/OSM.sol#93) is not in mixedCase
Parameter OSM.changePokeTimeout(uint256)._pokeTimeout (contracts/lmcv/OSM.sol#101) is not in mixedCase
Parameter PriceUpdater.updateSource(bytes32,address)._osm (contracts/lmcv/PriceUpdater.sol#77) is not in mixedCase
Parameter PriceUpdater.cage(uint256)._live (contracts/lmcv/PriceUpdater.sol#84) is not in mixedCase
Parameter RatesUpdater.changeStabilityRate(uint256)._stabilityRate (contracts/lmcv/RatesUpdater.sol#103) is not in mixedCase
Contract ddPRIMELike (contracts/staking/StakingVault.sol#16-18) is not in CapWords
Parameter StakingVault.bytes32ToString(bytes32)._bytes32 (contracts/staking/StakingVault.sol#322) is not in mixedCase
Variable StakingVault.RewardTokenList (contracts/staking/StakingVault.sol#35) is not in mixedCase
Variable StakingVault.RewardData (contracts/staking/StakingVault.sol#36) is not in mixedCase
Contract ddPrime (contracts/staking/ddPrime.sol#7-204) is not in CapWords
Function ddPrime.DOMAIN_SEPARATOR() (contracts/staking/ddPrime.sol#57-59) is not in mixedCase
Constant ddPrime.version (contracts/staking/ddPrime.sol#13) is not in UPPER_CASE_WITH_UNDERSCORES
Variable ddPrime._DOMAIN_SEPARATOR (contracts/staking/ddPrime.sol#29) is not in mixedCase
Contract ddPrimeLike (contracts/staking/ddPrimeJoin.sol#7-10) is not in CapWords
Contract ddPrimeJoin (contracts/staking/ddPrimeJoin.sol#16-61) is not in CapWords
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#conformance-to-solidity-naming-conventions

getLatestPrice() should be declared external:
        - ChainlinkClient.getLatestPrice() (contracts/lmcv/ChainlinkClient.sol#19-29)
bytes32ToString(bytes32) should be declared external:
        - StakingVault.bytes32ToString(bytes32) (contracts/staking/StakingVault.sol#322-332)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#public-function-that-could-be-declared-external

Slither correctly flagged that:

some functions can be defined as external
missing zero address checks

Those issues are included in the findings section of the report.

No major issues found by Slither.

Halborn strongly recommends conducting a follow-up assessment of the project either within six months or immediately following any material changes to the codebase, whichever comes first. This approach is crucial for maintaining the project’s integrity and addressing potential vulnerabilities introduced by code modifications.

1. Introduction
2. Audit summary
3. Test approach & methodology
4. Scope
5. Risk methodology
6. Scope
7. Assessment summary & findings overview
8. Findings & Tech Details

8.1 User funds may get locked in join contracts
8.2 Users may not be able to unstake or claim rewards when stakedamountlimit is decreased
8.3 Integer underflows in stakingvault module
8.4 Data returned from chainlink is not validated
8.5 Administrators are allowed to burn users ddprime tokens without authorization
8.6 Contracts might lose administrator functionality
8.7 Improper role-based access control
8.8 Missing pause/unpause functionality
8.9 Missing data validation
8.10 Missing zero address checks
8.11 Missing events on changes
8.12 Require message not updated
8.13 Function could be defined as external
8.14 Missing natspec documentation
8.15 Variables could be defined as immutable
8.16 Changing public constant variables to private can save gas

9. Automated Testing

// Download the full report

* Use Google Chrome for best results

** Check "Background Graphics" in the print settings if needed

LMCV part 2

DAMfinance

Table of Contents

1. INTRODUCTION

2. AUDIT SUMMARY

3. TEST APPROACH & METHODOLOGY

4. SCOPE

5. RISK METHODOLOGY

6. SCOPE

7. Assessment Summary & Findings Overview

8. Findings & Tech Details

8.1 USER FUNDS MAY GET LOCKED IN JOIN CONTRACTS

Description

Code Location

contracts/staking/RewardJoin.sol

contracts/staking/RewardJoin.sol

contracts/staking/StakeJoin.sol

Score

Recommendation

8.2 USERS MAY NOT BE ABLE TO UNSTAKE OR CLAIM REWARDS WHEN stakedAmountLimit IS DECREASED

Description

Code Location

contracts/staking/StakingVault.sol

contracts/staking/StakingVault.sol

Score

Recommendation

8.3 INTEGER UNDERFLOWS IN STAKINGVAULT MODULE

Description

Code Location

contracts/staking/StakingVault.sol

staking/StakingVault.sol

staking/StakingVault.sol

Score

Recommendation

8.4 DATA RETURNED FROM CHAINLINK IS NOT VALIDATED

Description

Code Location

contracts/lmcv/ChainlinkClient.sol

Score

Recommendation

8.5 ADMINISTRATORS ARE ALLOWED TO BURN USERS DDPRIME TOKENS WITHOUT AUTHORIZATION

Description

Code Location

staking/ddPrime.sol

Score

Recommendation

8.6 CONTRACTS MIGHT LOSE ADMINISTRATOR FUNCTIONALITY

Description

Code Location

Score

Recommendation

8.7 IMPROPER ROLE-BASED ACCESS CONTROL

Description

Score

Recommendation

8.8 MISSING PAUSE/UNPAUSE FUNCTIONALITY

Description

Code Location

Score

Recommendation

8.9 MISSING DATA VALIDATION

Description

Code Location

lmcv/Liquidator.sol

Score

Recommendation

8.10 MISSING ZERO ADDRESS CHECKS

Description

Score

8.11 MISSING EVENTS ON CHANGES

Description

Code Location

Score

Recommendation

8.12 REQUIRE MESSAGE NOT UPDATED

Description

Score

8.13 FUNCTION COULD BE DEFINED AS EXTERNAL

Description

Code Location