# B-Protocol Short Audit

## 1 - Executive Summary

Wil Barnes and Lucas Manuel have conducted an audit of a set of B-Protocol's smart contracts, in the context of confirming the safety of users' funds. B-Protocol is a platform that aims to provide users with an interface to deposit funds into lending platforms, giving priority of their liquidations to a defined set of liquidators. The contracts that were audited were interfacing with MakerDAO's core lending contracts, mainly `vat.sol`.

B-Protocol inherits one of MakerDAO's contracts to perform these actions on the Vat, `DssCdpManager.sol`, which has been in production since November 2019. Using the contracts in B-Protocol, the user is able to deposit and withdraw funds in the same way that they would when using MakerDAO's contracts, with the added benefit of sharing profit with liquidators.

Liquidators have early access to user vaults, only being able to `topup()` funds, preventing the user from being liquidated when the next OSM price is poked. Performing this action allows liquidators to have rights to liquidate that vault. After topping up a vault, a liquidator (normal or malicious) can only call `untopByPool()` and `bite()`.

When a user vault is vulnerable, a liquidator (normal or malicious) only can perform three actions: `topup()`, `untopByPool()`, and `bite()`. Malicious liquidators do not have access to anything other than user's on-auction debt.

The review was conducted over the course of two days, September 19th and 20th, 2020.

## 2 - Scope

The scope of this audit was to confirm the safety of user's funds in B-Protocol in their relation to interactions with MakerDAO's core protocol. This review focused on commit hash `c04dc240f9abfd96d5f7fc885e8a7099b3b5464c`. The list of files that were reviewed can be found in *Appendix 1 - Files in Scope*.

### 2.1 - Objectives

The following objectives were outlined in the [B-Protocol (v1) smart contract specification and security guarantees](https://docs.google.com/document/d/1aUxc5TX0cniql6yb5ieqWggebAqk0r4QtIhXCIlnQ3w/edit#heading=h.mbrlbfg68o7s) document that was provided at the beginning of the audit:

*The user interacts with our alternative CDP system, but potentially could interact directly with makerdao vault, and the following guarantees should hold:*
1. *Given a user sum of deposits and withdrawals (via the CDP or directly to the vat), and current ethereum timestamp, the user could get liquidated on B-Protocol, if and only if, he could get liquidated on makerdao.*
2. *Assuming the upgradable user score contract is not malicious: If a user was never liquidated, then regardless of any upgrade in makerdao (excluding the vat.sol module), he get/pay exactly the same by interacting with B-Protocol vs interacting with makerdao (i.e., security is the same). Even if the pool contract is malicious.*
3. *If the user score contract is malicious/gas consuming, then (2) still applies if the user calls quitB(...) when the malicious upgrade is being done.*
4. *User does get liquidated, he get the same conditions as in makerdao, under the assumption that:*
5. *Admin has no control over user deposited funds. Any upgrade to score or pool contracts will not affect user funds.*

The correctness of the user score and proceeds distribution is not in the scope of this audit. And in particular, if the liquidators collude, the distribution is not guaranteed at this point.

### 2.2 - Results and Observations
- Code quality was high, and appears to have good test coverage.
- A small error in one of the comments was found, leading to some potential confusion: [in line 202](https://github.com/backstop-protocol/dss-cdp-manager/blob/c04dc240f9abfd96d5f7fc885e8a7099b3b5464c/src/pool/Pool.sol#L202) on `Pool.sol` a 48 hour time lock is mentioned, but we wanted to point out that the current governance delay is 12 hours (see Misc section here: https://catflip.co/).
- The inheritance structure of `BCdpManager.sol` prevents users from calling `frob()` maliciously (without calling `untop()` and `updateScore()`). Users can still use the `flux()` and `move()` functions as these do not interact with `dink` or `dart`.
- The `Pool` contract could be upgraded to a malicious implementation that could pass in a high `uint` value to `topup()`, which has the potential to put the CDP in a less safe state. To mitigate this, it is recommended to use `toInt()` on [line 74](https://github.com/backstop-protocol/dss-cdp-manager/blob/c04dc240f9abfd96d5f7fc885e8a7099b3b5464c/src/LiquidationMachine.sol#L74) in `LiquidationMachine.sol`.

Below is a comprehensive list of findings in relation to each of the core objectives that were outlined for the scope of this audit.

*The user interacts with our alternative CDP system, but potentially could interact directly with makerdao vault, and the following guarantees should hold:*
1. *Given a user sum of deposits and withdrawals (via the CDP or directly to the vat), and current ethereum timestamp, the user could get liquidated on B-Protocol, if and only if, he could get liquidated on makerdao.*
- **If a liquidator determines that a CDP in B-Protocol will be unsafe the next time that the OSM price is updated (using`peek()` to check), they can `topup()` the CDP. This action saves the CDP from being liquidated in the MakerDAO system because some of the debt is temporarily paid off.**
- **The way that `Pool.sol` is currently designed, liquidators can only `topup()` CDPs that will be considered unsafe, topping them up with an amount of debt that will save their vault from being liquidated. If the `Pool` contract was upgraded to a malicious implementation, a liquidator could potentially pass in a large `uint` value that would cast to a negative integer, allowing a liquidator to make a user's vault less safe. It is recommended that B-Protocol use the `toInt()` function in the `topup()` function to `frob()` on [line 74](https://github.com/backstop-protocol/dss-cdp-manager/blob/c04dc240f9abfd96d5f7fc885e8a7099b3b5464c/src/LiquidationMachine.sol#L74) in `LiquidationMachine.sol`. In reality, the `vat.move()` call on [line 73](https://github.com/backstop-protocol/dss-cdp-manager/blob/c04dc240f9abfd96d5f7fc885e8a7099b3b5464c/src/LiquidationMachine.sol#L73) would fail because the number that would be used would be higher than the `pool`'s balance of `dai`, but this measure should still be taken to ensure the safety of users' CDPs.**
- **Except for this one edge case, point (1) holds true.**
2. *Assuming the upgradable user score contract is not malicious: If a user was never liquidated, then regardless of any upgrade in makerdao (excluding the vat.sol module), he get/pay exactly the same by interacting with B-Protocol vs interacting with makerdao (i.e., security is the same). Even if the pool contract is malicious.*
- **When interacting with B-Protocol, the user will interact with the Maker Protocol's Vat contract in the same way that they would if they used Maker's contracts directly, but the user will pay more gas for transactions because of `updateScore()` and `untop()` function calls.**
3. *If the user score contract is malicious/gas consuming, then (2) still applies if the user calls quitB(...) when the malicious upgrade is being done.*
- **If a user wants to opt out of B-Protocol's functionality but maintain their CDP in the context of the Maker protocol, the `quitB()` function allows user to not participate in liquidation aspects of B protocol, so point (2) still applies.**
4. *User does get liquidated, he get the same conditions as in makerdao, under the assumption that:*
- *The price feed reflects the real market price*
- *1 hour has elapsed (in makerdao it is currently 6 hours, but might change in the future).*
- **B-Protocol does not use a `dunk` parameter to limit that amount of collateral that can be sent for auction in a given CDP. This means that an entire CDP can be bitten with one function call. This is not that much different from how MakerDAO operates however, as multiple `bite()` function calls can be made in the same transaction, achieving the same effect. Other than this small difference, the user would get the same conditions as using MakerDAO's contracts directly.**
- **NOTE: We relayed the differences between B-Protocol and MCD liquidation processes, specifically the `dunk` and `box` parameters, and Yaron explained that they do not intend on matching that auction mechanism in B-Protocol.**
5. *Admin has no control over user deposited funds. Any upgrade to score or pool contracts will not affect user funds.*
- **Pool contract can only call: `topup()`, `untopByPool()`, and `bite()`. `topup()` can only make a CDP safer, and `bite()` can only be called when a CDP is eligible for liquidation. `untopByPool()` can be called by a malicious `Pool` to trigger a liquidation on a CDP in the Maker Protocol that could have been liquidated by B-Protocol, so the user would miss out on the chop savings.**
- **The score contract only affects a user's potential claims on `userDink`, so a malicious score contract would not be able to steal funds from a user’s CDP.**
- **In the most adverse situation, a user’s funds is liquidated by MCD keepers in the same way that they would have if they had interacted with the Maker Protocol's contracts directly, so B-Protocol's contracts do not cause any additional liquidation risk to users.**

***BProxyActions.sol***

This contract inherits an unchanged DssProxyActions, and adds 4 functions: `shiftManager()`, `lockETHViaCdp()`, `openLockEthAndGiveToProxy`, and `openAndImportFromManager()`. These functions use existing `dss-cdp-manager` functions. As long as users do not `cdpAllow()` or `urnAllow()` to external contracts, the contracts would have the same degree of security that the inherited contracts have.

## Appendix 1 - Files in Scope
- `BCdpManager.sol`
- `LiquidationManager.sol`
- `pool/Pool.sol`
- `DssCdpManager.sol` (unchanged from MCD)
- `BProxyActions.sol`
- `DssProxyActions.sol` (unchanged from MCD)

## Appendix 2 - Artifacts
Artifacts used
Included here is the [Surya tool](https://github.com/ConsenSys/surya) call graph of BCdpManager.

## Appendix 3 - Disclosure
- This short audit report is not an opinion of the Maker Foundation.