Read the findings of the 2022 Web3 Security Report

In the contracts `CToken/Unitroller/CErc20Delegator`, the role `admin` has the authority over the following function:

- `_setComptroller()`: change the implementation of `Comptroller` with any contracts,
- `_setPendingImplementation()/_acceptImplementation()`: change the implementation of `Unitroller` with any contracts,
- `_setImplementation()`: change the implementation of `CErc20` with any contracts,

Any compromise to the `admin` account may allow the hacker to take advantage of this and users' assets may suffer loss.

Centralization related risks

A serious issue was caused by Compound’s centralized oracle solution which pulls market data from only a single exchange, Coinbase, with Uniswap TWAP used as a backstop. 

Using Uniswap TWAP as a backstop is better than no backstop in this situation, but it introduces a false sense of security as it too can trivially be manipulated (as we saw during this event). 

Price oracle feed

The given input is missing the check for the non-zero address.

For example,
- contract `Comptroller`: `newPauseGuardian` in function `_setPauseGuardian()`,
- contract `CToken`: `newPendingAdmin` in function `_setPendingAdmin()`,
- contract `Unitroller`: `newPendingImplementation` in function `_setPendingImplementation()`, `newPendingAdmin` in function `_setPendingAdmin()`

Missing input validation

The compiler version utilized throughout the project uses the "^" prefix specifier, denoting that a compiler version which is greater than the version will be used to compile the contracts. 

Unlocked compiler version declaration

“public” functions that are never called by the contract should be declared “external”. When the inputs are arrays, “external” functions are more efficient than “public” functions.

Examples: 

Functions like : 

- contract `Comptroller`: `enterMarkets()`, `getAccountLiquidity()`, `getHypotheticalAccountLiquidity()`, `_setPriceOracle()`, `_setPauseGuardian()`, `_setMintPaused()`, `_setBorrowPaused()`, `_setTransferPaused()`, `_setSeizePaused()`, `_become()`, `claimComp()`, `getAllMarkets()`, 
- contract `CToken`: `initialize()`, `_setInterestRateModel()`,
- contract `Comp`: `delegate()`, `delegateBySig()`, `getPriorVotes()`
- contract `WJST`: `deposit()`, `withdraw()`, `getPriorVotes()`, `voteFresh()`, `withdrawVotes()`, `setGovernorAlpha()`, `transferOwnership()`,
- contract `Unitroller`: `_setPendingImplementation()`, `_setPendingAdmin()`, `_acceptAdmin()`,
- contract `CErc20`: `initialize()`,
- contract `CCErc20Delegate`: `_becomeImplementation()`, `_resignImplementation()`,
- contract `CErc20Delegator`: `borrowBalanceStored()`, `exchangeRateCurrent()`, `exchangeRateStored()`, `accrueInterest()`, `_setComptroller()`, `_setInterestRateModel()`,
- contract `GovernorAlpha`, `propose()`.

Proper usage of “public” and “external” type

Solidity defines a naming convention that should be followed. In general, the following naming conventions should be utilized in a Solidity file:

Constants should be named with all capital letters with underscores separating words UPPER_CASE_WITH_UNDERSCORES

refer to https://solidity.readthedocs.io/en/v0.5.17/style-guide.html#naming-conventions

Examples:
 
Constants like : 

- contract `CTokenStorage`: `borrowRateMaxMantissa`, `reserveFactorMaxMantissa`,
- contract `CTokenInterface`: `isCToken`,
- contract `Comptroller`: `compClaimThreshold`,  `compInitialIndex`, `closeFactorMinMantissa`, `closeFactorMaxMantissa`, `collateralFactorMaxMantissa`, `liquidationIncentiveMinMantissa`, `liquidationIncentiveMaxMantissa`,
- contract `ComptrollerInterface`: `isComptroller`,
- contract `Exponential` `expScale`, `doubleScale`, `halfExpScale`, `mantissaOne`,
- contract `InterestRateModel`: `isInterestRateModel`,
- contract `PriceOracle`: `isPriceOracle`,

 Incorrect naming convention utilization 

Boolean constants in code have only a few legitimate uses. Other uses (in complex expressions, as conditionals) indicate either an error or, most likely, the persistence of faulty code.

For example: 

```solidity
        if (false) {
            maxAssets = maxAssets;
        }
```

Misuse of a boolean constant

The return value of an external call is not stored in a local or state variable.

Examples: 

```solidity
function _supportMarket(CToken cToken) external returns (uint) {
  ...
  cToken.isCToken();
  ...
}
```


Return value not stored

Boolean constants can be used directly and do not need to be compared to true or false.

For example:

```solidity
        if (marketToJoin.accountMembership[borrower] == true) {
            // already joined
            return Error.NO_ERROR;
        }
```

Boolean equality

The following codes in the functions `redeemFresh()` and `borrowFresh()` do not meet the Checks-Effects-Interactions pattern.

```solidity=704
        doTransferOut(redeemer, vars.redeemAmount);

        /* We write previously calculated values into storage */
        totalSupply = vars.totalSupplyNew;
        accountTokens[redeemer] = vars.accountTokensNew;
```

```solidity=800
        doTransferOut(borrower, borrowAmount);

        /* We write the previously calculated values into storage */
        accountBorrows[borrower].principal = vars.accountBorrowsNew;
        accountBorrows[borrower].interestIndex = borrowIndex;
        totalBorrows = vars.totalBorrowsNew;
```

It only has a reentrancy lock as there is no lock at the controller level, only the CToken level.

If the `cToken` is an ERC777 protocol, the reentrancy can happen in function levels of an ERC777 based contract, i.e. multiple function calls that are triggered by the hook mechanism of ERC777.

This issue is possible to happen with all compound forks, but Compound is not affected as they do not list tokens with callback functionality.

Checks-Effects-Interactions pattern violations

In the aforementioned line, the formula for the calculation of `exchangeRate` is as follows after cToken is minted:

$$
exchangeRate =\\
 {totalCash + totalBorrows - totalReserves \over totalSupply }
$$


```solidity=344
    function exchangeRateStoredInternal() internal view returns (MathError, uint) {
        uint _totalSupply = totalSupply;
        if (_totalSupply == 0) {
            /*
             * If there are no tokens minted:
             *  exchangeRate = initialExchangeRate
             */
            return (MathError.NO_ERROR, initialExchangeRateMantissa);
        } else {
            /*
             * Otherwise:
             *  exchangeRate = (totalCash + totalBorrows - totalReserves) / totalSupply
             */
            uint totalCash = getCashPrior();
            uint cashPlusBorrowsMinusReserves;
            Exp memory exchangeRate;
            MathError mathErr;

            (mathErr, cashPlusBorrowsMinusReserves) = addThenSubUInt(totalCash, totalBorrows, totalReserves);
            if (mathErr != MathError.NO_ERROR) {
                return (mathErr, 0);
            }

            (mathErr, exchangeRate) = getExp(cashPlusBorrowsMinusReserves, _totalSupply);
            if (mathErr != MathError.NO_ERROR) {
                return (mathErr, 0);
            }

            return (MathError.NO_ERROR, exchangeRate.mantissa);
        }
    }
```

In solidity, division calculations have truncation problems. The `totalSupply` will be 1 and `exchangeRate` will be much smaller than `initialExchangeRate` in case the last user redeems (`accountTokens[redeemer] - 1`) cToken.

As a result, the `exchangeRate` would be extremely small.

When the value of `exchangeRate` is much smaller than `initialExchangeRate`, the user can mint cTokens well above normal values, and then the value of `exchangeRate` will be normal with the interest generating. In other words, the users can use this arbitrage to take away the underlying tokens in this pool.

For example, the user can mint the amount of 1e8 cToken with one underlying token in case `exchangeRate = 1/1e8`.

Logical issue of function `exchangeRateStoredInternal()`

In the contract `GovernorAlpha` the role `guardian` has authority over the functions shown in the diagram below.

- `cancel()`, to cancel the proposal.
- `__acceptAdmin()`, to accept admin of the `Timelock` contract.
- `__abdicate()`, to renounce `guardian`.
- `__queueSetTimelockPendingAdmin()`, to queue the transaction for `Timelock.setPendingAdmin()`.
- `__executeSetTimelockPendingAdmin()`, to execute the transaction for `Timelock.setPendingAdmin()`.

Any compromise to the `guardian` account may allow the hacker to take advantage of this authority.

![](https://accelerator-tasks-prod.s3.amazonaws.com/d443422032dc4e25897401782494a0b0/diagrams/centralization-GovernorAlpha-guardian.svg)

In the contracts `PriceOracle`, the role `anchorAdmin` has the authority over the following function:

- `_setPendingAnchor()`: set the anchor price for an asset,
- `_setPaused()`: pause or resume the market,

Any compromise to the `anchorAdmin` account may allow the hacker to take advantage of this and users' assets may suffer loss.

In the contracts `PriceOracle`, the role `poster` has the authority over the following function:

- `setPrice()`: set price for an asset,
- `setPrices()`:  set prices for a variable number of assets

Any compromise to the `poster` account may allow the hacker to take advantage of this and users' assets may suffer loss.

In the function `setPriceInternal()`, it will not save the posted price when it exceeds the maximum swing(currently: 10%). So it cannot guarantee that the posted price is between the `price - max swing` and `price + max swing`, and the posted price could be changed to the max price when it exceeds the max swing. The real asset price may exceed the maximum swing.

```solidity=760
if (localVars.pendingAnchorMantissa != 0) {
	// let's explicitly set to 0 rather than relying on default of declaration
	localVars.anchorPeriod = 0;
	localVars.anchorPrice = Exp({mantissa : localVars.pendingAnchorMantissa});

	// Verify movement is within max swing of pending anchor (currently: 10%)
	(err, localVars.swing) = calculateSwing(localVars.anchorPrice, localVars.price);
	if (err != Error.NO_ERROR) {
		return failOracleWithDetails(asset, OracleError.FAILED_TO_SET_PRICE, OracleFailureInfo.SET_PRICE_CALCULATE_SWING, uint(err));
	}

	// Fail when swing > maxSwing
	if (greaterThanExp(localVars.swing, maxSwing)) {
		return failOracleWithDetails(asset, OracleError.FAILED_TO_SET_PRICE, OracleFailureInfo.SET_PRICE_MAX_SWING_CHECK, localVars.swing.mantissa);
	}
} else {
	localVars.anchorPeriod = anchors[asset].period;
	localVars.anchorPrice = Exp({mantissa : anchors[asset].priceMantissa});

	if (localVars.anchorPeriod != 0) {
		(err, localVars.priceCapped, localVars.price) = capToMax(localVars.anchorPrice, localVars.price);
		if (err != Error.NO_ERROR) {
			return failOracleWithDetails(asset, OracleError.FAILED_TO_SET_PRICE, OracleFailureInfo.SET_PRICE_CAP_TO_MAX, uint(err));
		}
		if (localVars.priceCapped) {
			// save for use in log
			localVars.cappingAnchorPriceMantissa = localVars.anchorPrice.mantissa;
		}
	} else {
		// Setting first price. Accept as is (already assigned above from requestedPriceMantissa) and use as anchor
		localVars.anchorPrice = Exp({mantissa : requestedPriceMantissa});
	}
}
```


Logical issue of `setPriceInternal()`

In the contract `WJST` the role `_owner` has authority over the functions shown in the diagram below.

- `setGovernorAlpha()`, to set the `GovernorAlphaInterface` contract.
- `transferOwnership()`, to transfer ownership to another account.

Any compromise to the `_owner` account may allow the hacker to take advantage of this authority.

![](https://accelerator-tasks-prod.s3.amazonaws.com/d443422032dc4e25897401782494a0b0/diagrams/centralization-WJST-_owner.svg)

The users will deposit `JST` token to mint `WJST`. They need to lock `WJST` token to the `WJST` contract to vote for a proposal. If there are multiple active proposals, the user's votes are locked in the voted proposal, but cannot vote for the others.


Vote for Multiple Active Proposals

Justlend

Reservoir.sol

CErc20Delegator.sol

Maximillion.sol

CTokenInterfaces.sol

Exponential.sol

Timelock.sol

PriceOracle.sol

CErc20Delegate.sol

WhitePaperInterestRateModel.sol

ComptrollerG2.sol

ErrorReporter.sol

JumpRateModel.sol

SimplePriceOracle.sol

ComptrollerInterface.sol

ComptrollerG1.sol

EIP20Interface.sol

PriceOracleProxy.sol

EIP20NonStandardInterface.sol

Lens

JumpRateModelV2.sol

ComptrollerStorage.sol

CEther.sol

CompoundLens.sol

GovernorAlpha.sol

CToken.sol

CarefulMath.sol

InterestRateModel.sol

DAIInterestRateModelV2.sol

DSValue.sol

CErc20Immutable.sol

PriceOracle

BaseJumpRateModelV2.sol

CErc20.sol

WJST.sol

Comptroller.sol

Unitroller.sol

SafeMath.sol

CDaiDelegate.sol

JustLend  

TRON Founder Justin Sun on Crypto Contagion Concerns

Huobi Token Surges 75% as Tron Founder Justin Sun Calls for Empowering the Exchange Token

Report: Tether freezes $46M of FTX’s USDT on Tron blockchain, setting new precedent

: Tron founder prepared to provide ‘billions’ in aid to FTX

Justin Sun, Tron DAO line up to contribute to Binance recovery fund: Exclusive

Oryen Network Convinces Tamadoge And Tron Holders To Invest After A 100% Price Increase | Bitcoinist.com

Harmony Hackers Cover Tracks by Bridging Portion of $100M Loot to Avalanche, Ethereum and Tron

Solana (SOL) Price Down By -1.6% With Tron (TRX) Down 4.8%; Which Coin Beats Both? | NewsBTC

Buy Big Eyes (BIG), a meme coin creating a sustainable DeFi exceeding the efforts of Monero (XMR) and Tron (TRX) | NewsBTC

Tron Founder Justin Sun Ready To Invest $1 Billion On DCG Assets

Today in Crypto: FTX Resumes Employee Salaries, Huobi Partners with Tron & DMC on Dominica Coin, Ukrainian & Indian CBDCs

FTX Bear Market Sees Investors Flock To The Hideaways (HDWY), Selling Polkadot (DOT) and Tron (TRX) | NewsBTC

Justin Sun’s TRON DAO Buys $1B of Tether (USDT) as it Briefly Falls to $0.97

Can A Brand-New Cryptocurrency, Metacryp, Make A Mark In The Crypto Space While Cardano And Tron Are - Bitcoinist

Apecoin (APE) Flips Bearish Forecasts, Tron (TRX) Unable To As The Hideaways (HDWY) Continues with Presale Success | NewsBTC

Tron Founder Justin Sun's Address Moves Millions into USDC, What's Going On

First Mover Asia: Why Is Tron Founder Justin Sun Keeping Some of His Coins in Valkyrie Digital Assets?

Not Sure About Bitcoin? Try These Alts - Adirize DAO, TRON, and Ethereum | Bitcoinist.com

TRON, Avalanche, and Big Eyes Coin - Three Cryptocurrencies That Could Yield Massive Returns In The Ongoing Bear Market | NewsBTC

Crypto Market Update: Polygon (MATIC) and Tron (TRX) Both Lose to The Hideaways (HDWY) Despite Gains | NewsBTC

Invest in These Cryptos to Secure Your Future: HedgeUp (HDUP), Tron (TRX), And Bitcoin (BTC)

JustLend

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

16

0

11

3

2