Read the findings of the 2022 Web3 Security Report

The role `owner` has the authority over the following function:
- `setAddresses()`

Any compromise to the `owner` account may allow the hacker to take advantage of this and set the addresses of the projects, which may cause the break of the entire system.

Centralization Risk

The `assert` function should only be used to test for internal errors, and to check invariants. The `require` function should be used to ensure valid conditions, such as inputs, or contract state variables are met, or to validate return values from calls to external contracts.

Proper Usage of `require` And `assert` Functions

The linked `transfer()` invocations do not check the return value of the function call which should yield a `true` result in case of proper ERC-20 implementation.

Unchecked Value of ERC-20 `transfer()` Call

The linked variable declarations do not have a visibility specifier explicitly set.

Visibility Specifiers Missing

The `collToken` is set to `_wethTokenAddress` in the `setAddresses()` function. If only WETH is intended as collToken than it should be declared constant as we know the WETH address beforehand

Variable Declare as Immutable or Constant

The uint in the linked structs is defaulting to uint256 but if there is no need of using 256bit for certain variables, there's a way to safe on gas by tight-packing the variables and using lower bits version of uint like uint128. This would safe on storage space and thus saving on gas.

Optimizable Usage of `uint`

Does any user who opened the trove and sent some LUSD to Stability Pool, can close the trove? Or will the transaction fail if a user don't have minted amount of LUSD when closing trove?

Potentially Unable To Close Trove When Holding Less Than Minted Amount

Compares to a boolean constant.
Example:
```solidity=175
if (collTokenAddresses[collTokens[i].tokenAddress] == true) {
```

Compares to a Boolean Constant

When the contract is deployed, the following actions will be executed:
- 1 million tokens are sent to the `_initialSetupAddress`. 
- 10 million tokens are sent to the `_communityIssuanceAddress`. 
- 0.05 million tokens are sent to the `_lpRewardsAddress`. 
- 15 million tokens are sent to the `_ecosystemVestingAddress`. 
- 7.5 million tokens are sent to the `_teamVestingAddress`. 
- 4 million tokens are sent to the `_partnerVestingAddress`.
- 8.95 million tokens are sent to the `_treasuryAddress`.
- 17.5 million tokens are sent to the `_multisigAddress`.

This could be a centralization risk as the `owner` can distribute tokens without obtaining the consensus of the community.

Initial Token Distribution

In the contract `USDCUnipool` and `Unipool`, the role `owner` has the authority over the following function:
- `setParams()`

Any compromise to the `owner` account may allow the hacker to take advantage of this and set the parameters of the projects, which may cause the break of the entire system.

In the contract `CommunityIssuance`, the role `owner` has the authority over the following function:
- `setAddresses()`

Any compromise to the `owner` account may allow the hacker to take advantage of this and set the addresses of the projects, which may cause the break of the entire system.

The assigned values to address type variables `_collTokenAddress` should be verified as non-zero values to prevent error.

Missing Input Validation

`collToken` is expected to be WETH but it still accepts custom ERC20 address in setAddresses function. It should be made constant as WETH token address is known before deployment

Variable Could Be Declared as Constant

The linked code segments contain named return variables for functions that do not utilize them.

Redundant Named Return Variables

The linked function declarations contain explicitly named `return` variables that are not utilized within the function's code block.

Return Variable Utilization

The state variable `SECONDS_IN_ONE_YEAR` is never used.

Unused Variable

In the contract `LockupContractFactory`, the role `owner` has the authority over the following functions:
- `setLQTYTokenAddress()`

Any compromise to the `owner` account may allow the hacker to take advantage of this and set the LQTY token address of the project, which may cause the break of the entire system.

The assigned values to address type variables `_lqtyTokenAddress` and `_beneficiary` should be verified as non-zero values to prevent error.

Lack of input validation 

The assigned values to address type variables `_initialSetupAddress`, `_lpRewardsAddress`, `_multisigAddress`, `_ecosystemVestingAddress`, `_teamVestingAddress`, `_partnerVestingAddress` and `_treasuryAddress` should be verified as non-zero values to prevent error.

Code lack sanity check for ecrecover. Raw ecrecover function will yield the zero address for any incorrect signature.

Lack of sanity checks on ecrecover

In the contract `LQTYToken`, the role `owner` has the authority over the following functions:
- `addCommunityIssuanceAddress(address newCommunityIssuanceAddress)`
- `removeCommunityIssuanceAddress(address newCommunityIssuanceAddress)`
- `transferToNewCommunityIssuanceContract(address newCommunityIssuanceAddress, uint256 amount)`

Any compromise to the `owner` account may allow the hacker to take advantage of this and do the following:
- add community issuance address.
- remove community issuance address.
- transfer tokens to new community issuance address.

The `_isFirstYear()` function does not check whether the time passed a year or not, but rather checks whether the time passed half a year or not.

Mismatch of Function Name and Implementation

```solidity=56
string constant internal _NAME = "testDollar";
```
The above statement obviously uses a name in the test environment.

Typo

Naming conventions are powerful when adopted and used broadly. The use of different conventions can convey significant *meta* information that would otherwise not be immediately available.

Solidity defines a naming convention that should be followed.

- Contracts and libraries should be named using the CapWords style.
- Structs should be named using the CapWords style.
- Events should be named using the CapWords style. 
- Functions should use mixedCase.
- Function arguments should use mixedCase.
- Local and State Variable Names should use mixedCase.
- Constants should be named with all capital letters with underscores separating words.
- Enums, in the style of simple type declarations, should be named using the CapWords style.

Reference: https://docs.soliditylang.org/en/latest/style-guide.html#naming-conventions

Incorrect Naming Convention Utilization

In the contract `LQTYStaking`, the role `owner` has the authority over the following functions:
- `setXdoTokenAddress(address _xdoTokenAddress)`
- `setXUSDTokenAddress(address _xUSDTokenAddress)`
- `addAddressesForColl(address _collTokenAddress, address _troveManagerAddress, address _borrowerOperationsAddress, address _activePoolAddress)`
- `RemoveAddressesForColl(address _collTokenAddress, address _troveManagerAddress, address _borrowerOperationsAddress, address _activePoolAddress)`

Any compromise to the `owner` account may allow the hacker to take advantage of this and do the following:
- set xdo token address.
- set xusdt token address.
- add addresses for coll
- remove addresses for coll


In the contract `LQTYStaking`, the accounts in the `troveManagerAddresses` have the authority over the following function:
- `increaseF_Coll(address collTokenAddress, uint256 _CollFee)`

Any compromise to the accounts in the `troveManagerAddresses` may allow the hacker to take advantage of this and do the following:
- `increaseF_Coll(address collTokenAddress, uint256 _CollFee)`


In the contract `LQTYStaking`, the accounts in `borrowerOperationsAddresses` have the authority over the following function:
- increase `CollFeePerLQTYStaked`

Any compromise to the accounts in the `borrowerOperationsAddresses` may allow the hacker to take advantage of this and do the following:
- increase `XUSDFeePerLQTYStaked`
    

There are some comments mentioned `LUSD` instead of `XUSD`. 

The signature malleability is possible within the Elliptic Curve cryptographic system. An Elliptic Curve is symmetric on the X-axis, meaning two points can exist with the same `X` value. In the `r`, `s` and `v` representation this permits us to carefully adjust `s` to produce a second valid signature for the same `r`, thus breaking the assumption that a signature cannot be replayed in what is known as a replay-attack.

Susceptible to Signature Malleability

xDollar.fi

LQTYToken.sol

StabilityPool.sol

CommunityIssuance.sol

IPool.sol

SafeMath.sol

GasPool.sol

IUnipool.sol

ILQTYStaking.sol

StableCollTroveManager.sol

LockupContract.sol

LiquityMath.sol

ITroveManager.sol

Interfaces

StableCollBorrowerOperations.sol

LQTYStaking.sol

LiquityBase.sol

IBorrowerOperations.sol

LQTY

xDollar

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

120

0

15

73

32