In the contract `LPTokenBoardroom`, the role `DEFAULT_ADMIN_ROLE` has authority over the following functions:
- `setZapper()` to update the state variable `zapper`;
- `setCompoundEnabled()` to update the state variable `compoundEnabled`.

Any compromise to the `DEFAULT_ADMIN_ROLE` account may allow a hacker to take advantage of this authority and modify the behavior of the contract.

In the contract `Treasury`, the role `DEFAULT_ADMIN_ROLE` has authority over the following functions:
- `initialize()` to initialize the contract;
- `setSmartBondPool()` to update the state variable `smartBondPool`;
- `setSmartBondPoolPercent()` to update the state variable `bondRepayToBondSmartPoolPercent`;
- `setContractionIndex()` to update the state variable `contractionIndex`;
- `setExpansionIndex()` to update the state variable `expansionIndex`;
- `setBondRepayPercent()` to update the state variable `bondRepayPercent`;
- `setMaxSupplyContractionPercent()` to update the state variable `maxSupplyContractionPercent`;
- `setMaxDebtRatioPercent()` to update the state variable `maxDebtRatioPercent`;
- `setBondDepletionFloorPercent()` to update the state variable `bondDepletionFloorPercent`;
- `setDevPercentage()` to update the state variable `devPercentage`;
- `setDevAddress()` to update the state variable `devAddress`;
- `setDollarOracle()` to update the state variable `dollarOracle`;
- `setTriggerRebasePriceCeiling()` to update the state variable `triggerRebasePriceCeiling`;
- `setTriggerRebaseNumEpochFloor()` to update the state variable `dollarPriceCeiling`;
- `setDollarPriceCeiling()` to update the state variable `triggerRebaseNumEpochFloor`;
- `setSharesMintedPerEpoch()` to update the state variable `sharesMintedPerEpoch`;
- `migrate()` to handle migrating assets to a new treasury contract;
- `governanceRecoverUnsupported()` to withdraw the unsupported tokens;
- `addEpochListener()` to add a `IEpochListener` into the array `listeners`;
- `removeEpochListener()` to remove a `IEpochListener` from the array `listeners`.

Any compromise to the `DEFAULT_ADMIN_ROLE` account may allow a hacker to take advantage of this authority and modify the behavior of the contract. 
In case of compromise of the private key for this address, an attacker could use the `migrate()` function to withdraw all funds from the contract.

Centralization Related Risks

The contract approves the zapper an unlimited amount of `cash` to spend. However, when changing the zapper to a new one, it does not revoke the approval of the previous zapper. 

Exploit scenario:
- Suppose, `LPTokenBoardroom` mistakenly initializes the zapper as a malicious contract, which can drain `cash` from the `LPTokenBoardroom`;
- The owner changes the zapper by calling `setZapper()`;
- However, since the zapper still has an unlimited amount of allowance, it can keep draining `cash`.

Not Revoking the Approval of the Previous Zapper

A sandwich attack might happen when an attacker observes a transaction swapping tokens or adding liquidity without setting restrictions on slippage or minimum output amount. The attacker can manipulate the exchange rate by frontrunning (before the transaction being attacked) a transaction to purchase one of the assets and make profits by backrunning (after the transaction being attacked) a transaction to sell the asset.

The following function is called without setting restrictions on slippage or minimum output amount, so transactions triggering this function is vulnerable to sandwich attacks, especially when the input amount is large:

- `_swapShareToCash()`
```solidity=96
router.swapExactTokensForTokensSupportingFeeOnTransferTokens(
    amount,
    0,
    shareToCashPath,
    address(this),
    block.timestamp + 600
);
```

Potential Sandwich Attacks

An unlimited allowance is generally considered ill-practice when dealing with user funds.

In different places, it was found that unlimited allowances were configured.

File `LPTokenBoardroom.sol`:
```solidity=49
share.safeApprove(address(router), type(uint256).max);
```

```solidity=112
function _approveZapper() private {
    cash.safeApprove(address(zapper), type(uint256).max);
}
```


Potentially Dangerous Allowance

The function `allocateSeigniorage` is external, everyone can call it. \
By sending dollars (\$STATIC) to this function, the same amount of bond tokens (\$PULSE) is burnt.

By burning bonds, and reducing the corresponding amount of shares, this function allows an external party to manipulate the value of `accTokenPerShare`, which is used in reward calculation. \
Indeed, after having called this function, the new value will be: 
```solidity=87
accTokenPerShare = accTokenPerShare.mul(oldBondSupply).div(
    newBondSupply
);
```
This can have negative a negative impact. 

A scenario causing issue to the project could be as following:
- Suppose the initial state is:
  - The amount of bounds/shares in contract (`totalBalance()`) : 10 000.
  - Value of `accTokenPerShare` : 10.
  - This is the first time the user interacts with the contract (`user.deposit` = 0).
  - In order to simplify the calculations, `PRECISION_FACTOR` is equal to 10 (`decimalsRewardToken` is equal to 29).
```solidity=62
uint256 decimalsRewardToken = uint256(_rewardToken.decimals());
require(decimalsRewardToken < 30, 'Must be inferior to 30');

PRECISION_FACTOR = uint256(10**(uint256(30).sub(decimalsRewardToken)));
```

Then the attacker proceeds as following:
1. He deposits 1 000 bonds (\$PULSE) with the `deposit()` function:

- Value of `user.deposit` : 1 000 shares. (1 share received per bond) :
```solidity=107
user.deposit = user.deposit.add(_amount);
```

- Value of `user.rewardDebt` : 1 000.
```solidity=117
user.rewardDebt = balanceOf(msg.sender).mul(accTokenPerShare).div(
    PRECISION_FACTOR
);
```
  - New value of `totalBalance()` : 11 000. \
This is because the amount of bounds/shares in the contract is now 11 000.

2. He calls `allocateSeigniorage()` with 10 999 dollars \$STATIC, which will burn 10 999 bonds \$PULSE. \
This will be accepted because the `totalBalance()` of bonds/shares (11 000) is greater than the `amount_` of dollars sent (10 999). 
```solidity=68
function allocateSeigniorage(uint256 amount_) external override {
    require(amount_ > 0, "amount can't be zero");
    require(totalBalance() > 0, 'No money deposited');
    require(totalBalance() > amount_, 'Too many dollars for bonds');
```
- New value of `oldBondSupply` : 11 000.
- New value of `newBondSupply` : 1.
- Following the calculation below, the `accTokenPerShare` will become 110 000.
```solidity=87
accTokenPerShare = accTokenPerShare.mul(oldBondSupply).div(
    newBondSupply
);
```

3. When he calls `withdraw()`, the underlying `redeem()` call will be performed with the inflated value of `accTokenPerShare`.
```solidity=226
uint256 pendingRewards = userBondBalance // 1 000
    .mul(accTokenPerShare) // 110 000
    .div(PRECISION_FACTOR) // 10
    .sub(user.rewardDebt); // 1 000

if (pendingRewards > 0) {
    rewardToken.safeTransfer(_user, pendingRewards);
}
```

In this case, `userBondBalance` is the amount of shares for the user: 1 000.
```solidity=215
uint256 userBondBalance = balanceOf(_user);
```

This means the attacker would retrieve 10 990 000 `pendingRewards` tokens for having spent 1 000 dollars (\$PULSE).
Depending on the value of `rewardToken`, the attacker might be able to make a very large profit from the operation. 

Additionally, the profit can be amplified by flashloan. 
An attacker could borrow a significant amount of `dollarToken` (\$STATIC) and call `allocateSeigniorage()` to dramatically manipulate the value of `accTokenPerShare`.

Exploit scenario with flashloan:
- Attacker deposits some bound tokens (\$PULSE) into `SmartBondPool`.
- After some period, the attacker flashloans `dollarToken` (\$STATIC) and invokes `allocateSeigniorage()` to increase `accTokenPerShare`. In this case, the user's reward is calculated with the inflated value of `accTokenPerShare`.
- The attacker withdraws all the deposited tokens and earns a significant amount of `rewardToken`.
- The attacker sells the `rewardToken` and repays the flashloan.

Potential Reward Manipulation

The `Treasury` is serving as the underlying entity to interact with Oracles to acquire the price of \$STATIC. The Oracle implementation is injected during contract initialization and can be modified by the `DEFAULT_ADMIN_ROLE`. There are two major concerns to the Oracles:

1. Price acquired from oracles might be outdated:\
The function `_updateDollarPrice()` may not be called before calling the function `getDollarPrice()`. The issue could lead to receiving outdated price feed and affecting any related price calculation or display.

2. Price could be manipulated:\
The price could be manipulated by many reasons. For example, an attacker could borrow flashloan to temporarily manipulate the price. A "whale" can buy/sell \$STATIC in a short time period to manipulate the price.

The scope of the audit treats the Oracles as black boxes and assumes their functional correctness. However, in the real world, Oracles can be compromised and this may lead to lost or stolen assets. 

External Oracle Dependencies Risk

The function `migrate()` does not check if the target contract is different from the zero address.
If the owner calls the function with the zero address, all funds would be sent to the zero address and be permanently lost.

Lack of Zero Address Validation

The following functions are not void-returning functions. Ignoring their return values might cause unexpected exceptions:
* `IBasisAsset(bond).mint();`
* `IBasisAsset(dollar).mint();`
* `IBasisAsset(share).mint();`

Lack of Return Value Handling

The linked code implements a type conversion that directly converts type `int256` to `uint256`. 

For example,
```solidity=709
			_savedForBoardRoom = uint256(supplyDelta);
```
`supplyDelta` is an integer whose value can be EITHER positive OR negative. If the value of `supplyDelta` is negative, it could cause the value of `_savedForBoardRoom` to be an extremely big value.

Currently, the `supplyDelta` will always be positive due L644, where requiring the value of \$STATIC is bigger than ONE. Therefore, it will not cause any actual problem to the project. 

Charge DeFi

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

9

0

4

1

4