Prepared by:
HALBORN
Last Updated 04/26/2024
Date of Engagement by: September 8th, 2022 - September 29th, 2022
88% of all REPORTED Findings have been addressed
All findings
16
Critical
0
High
1
Medium
4
Low
4
Informational
7
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.
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.
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
)
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.
Critical
0
High
1
Medium
4
Low
4
Informational
7
Impact x Likelihood
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 |
// High
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.
cage
function:
function cage() external auth {
live = 0;
emit Cage();
}
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);
}
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");
});
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
// Medium
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 5000stakedAmountLimit
to 3000require
statementIn the above scenario, User 2 won't be able to unstake or claim the rewards until User 1 decreases his staked amount.
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:
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"));
});
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
// Medium
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.
The pullStakingToken
function is not verifying balance before subtraction in 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
:
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
:
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");
});
// Medium
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).
function getLatestPrice() public view returns (int256) {
(
/*uint80 roundID*/,
int256 price,
/*uint startedAt*/,
/*uint timeStamp*/,
/*uint80 answeredInRound*/
) = priceFeed.latestRoundData();
return price;
}
SOLVED: Validation of data returned from Chainlink was added.
Reference: ChainlinkClient.sol
// Medium
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.
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"));
});
PENDING: The DAMfinance team
stated that they plan to implement the recommended fix with the governance module in the future.
// Low
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.
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.
// Low
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.
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.
// Low
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.
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.
// Low
Configuration values in the contracts are not validated; it is possible to set large and/or invalid values, causing contracts to fail.
The pokeTimeout
has no maximum value and can be set to a very large value, making calling poke
impossible in a reasonable time.
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.
The lotSize
variable can be set to 0
, causing the liquidate
function to fail each time because of the require
statement:
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");
[...]
PARTIALLY SOLVED: The lotSize
validation was added in the Liquidator.sol contract.
However, the \client team
decided to leave other variables without validation.
// Informational
// Informational
Functions performing important changes on tchangePokeTimeout,
start,
stop,
void,
updateSource,
cage,
changeStabilityRate` are not emitting events to facilitate monitoring of the protocol.
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
// Informational
// Informational
SOLVED: Function definition was updated from public
to external
: ChainlinkClient.sol
// Informational
Solidity contracts can use a special form of comments to provide rich documentation for functions, return variables, and more. This special form is named the Ethereum Natural Language Specification Format (NatSpec).
PENDING: Natspec documentation is planned for future releases.
// Informational
Values of immutable variables can be set inside the constructor, but cannot be modified afterward.
The PriceUpdater
and RateUpdater
contracts contain the lmcv
variable, which is set only in the constructor and never updated. The such variable could be defined as immutable
similar to other LMCV contracts.
LMCVLike public lmcv; // CDP Engine
LMCVLike public lmcv; // LMCV contract
SOLVED: Variable definitions were updated:
// Informational
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.
uint256 constant RAY = 10 ** 27;
uint256 constant ONE_HOUR = 3600;
uint256 constant RAY = 10 ** 27;
SOLVED: Constant definitions were updated:
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.
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.
// Download the full report
* Use Google Chrome for best results
** Check "Background Graphics" in the print settings if needed