Fuse fixed accounting

.gitignore

@@ -39,3 +39,5 @@ junit.xml
 .build
 .last_confs
 .saddle_history
+artifacts
+cache

contracts/CEther.sol

@@ -135,8 +135,7 @@ contract CEther is CToken, CEtherInterface {
 
     function doTransferOut(address payable to, uint amount) internal {
         // Send the Ether and revert on failure
-        (bool success, ) = to.call.value(amount)("");
-        require(success, "doTransferOut failed");
+        to.transfer(amount);
     }
 
     function requireNoError(uint errCode, string memory message) internal pure {

contracts/CEtherDelegateTempExploitAccounting.sol

@@ -0,0 +1,59 @@
+pragma solidity 0.5.17;
+
+import "./CEtherDelegate.sol";
+
+/**
+ * @title Compound's CEtherDelegate Contract
+ * @notice CTokens which wrap Ether and are delegated to
+ * @author Compound
+ */
+contract CEtherDelegateTempExploitAccounting is CEtherDelegate {
+    /**
+     * @notice Called by the delegator on a delegate to initialize it for duty
+     * @param data The encoded bytes data for any initialization
+     */
+    function _becomeImplementation(bytes calldata data) external {
+        require(msg.sender == address(this) || hasAdminRights(), "!self");
+        require(accrueInterest() == uint(Error.NO_ERROR), "!accrue");
+
+        // Get secondary accounts from data
+        (address[] memory secondaryAccounts) = abi.decode(data, (address[]));
+        uint256 secondaryAccountsBorrowBalance = 0;
+
+        for (uint256 i = 0; i < secondaryAccounts.length; i++) {
+            address secondaryAccount = secondaryAccounts[i];
+
+            // Get account #2 borrow balance
+            uint256 secondaryAccountBorrowBalance = div_(mul_(accountBorrows[secondaryAccount].principal, borrowIndex), accountBorrows[secondaryAccount].interestIndex);
+            secondaryAccountsBorrowBalance = add_(secondaryAccountsBorrowBalance, secondaryAccountBorrowBalance);
+
+            // Set account #2 borrow balance to 0
+            accountBorrows[secondaryAccount].principal = 0;
+            accountBorrows[secondaryAccount].interestIndex = borrowIndex;
+        }
+
+        // Get account #1 supply balance
+        uint256 account1SupplyShares = accountTokens[0x32075bAd9050d4767018084F0Cb87b3182D36C45];
+        uint256 account1SupplyBalance = mul_ScalarTruncate(Exp({mantissa: exchangeRateStored()}), account1SupplyShares);
+
+        // Set account #1 supply shares to 0
+        accountTokens[0x32075bAd9050d4767018084F0Cb87b3182D36C45] = 0;
+
+        // Set account #1 borrow balance = secondary accounts' borrow balance - account #1 supply balance
+        require(secondaryAccountsBorrowBalance >= account1SupplyBalance, "Expect secondary accounts' combined borrow balance >= account #1 supply balance.");
+        require(accountBorrows[0x32075bAd9050d4767018084F0Cb87b3182D36C45].principal == 0, "Expect account #1 borrow balance to start at 0.");
+        accountBorrows[0x32075bAd9050d4767018084F0Cb87b3182D36C45].principal = sub_(secondaryAccountsBorrowBalance, account1SupplyBalance);
+        accountBorrows[0x32075bAd9050d4767018084F0Cb87b3182D36C45].interestIndex = borrowIndex;
+
+        // Subtract from total supply
+        totalSupply = sub_(totalSupply, account1SupplyShares);
+
+        // Subtract from total borrows
+        totalBorrows = sub_(totalBorrows, account1SupplyBalance);
+    }
+
+    /**
+     * @notice Function called before all delegator functions
+     */
+    function _prepare() external payable {}
+}

contracts/CToken.sol

@@ -703,6 +703,10 @@ contract CToken is CTokenInterface, Exponential, TokenErrorReporter {
         // EFFECTS & INTERACTIONS
         // (No safe failures beyond this point)
 
+        /* We write previously calculated values into storage */
+        totalSupply = vars.totalSupplyNew;
+        accountTokens[redeemer] = vars.accountTokensNew;
+
         /*
          * We invoke doTransferOut for the redeemer and the redeemAmount.
          *  Note: The cToken must handle variations between ERC-20 and ETH underlying.
@@ -711,10 +715,6 @@ contract CToken is CTokenInterface, Exponential, TokenErrorReporter {
          */
         doTransferOut(redeemer, vars.redeemAmount);
 
-        /* We write previously calculated values into storage */
-        totalSupply = vars.totalSupplyNew;
-        accountTokens[redeemer] = vars.accountTokensNew;
-
         /* We emit a Transfer event, and a Redeem event */
         emit Transfer(redeemer, address(this), vars.redeemTokens);
         emit Redeem(redeemer, vars.redeemAmount, vars.redeemTokens);
@@ -803,6 +803,11 @@ contract CToken is CTokenInterface, Exponential, TokenErrorReporter {
         // EFFECTS & INTERACTIONS
         // (No safe failures beyond this point)
 
+        /* We write the previously calculated values into storage */
+        accountBorrows[borrower].principal = vars.accountBorrowsNew;
+        accountBorrows[borrower].interestIndex = borrowIndex;
+        totalBorrows = vars.totalBorrowsNew;
+
         /*
          * We invoke doTransferOut for the borrower and the borrowAmount.
          *  Note: The cToken must handle variations between ERC-20 and ETH underlying.
@@ -811,11 +816,6 @@ contract CToken is CTokenInterface, Exponential, TokenErrorReporter {
          */
         doTransferOut(borrower, borrowAmount);
 
-        /* We write the previously calculated values into storage */
-        accountBorrows[borrower].principal = vars.accountBorrowsNew;
-        accountBorrows[borrower].interestIndex = borrowIndex;
-        totalBorrows = vars.totalBorrowsNew;
-
         /* We emit a Borrow event */
         emit Borrow(borrower, borrowAmount, vars.accountBorrowsNew, vars.totalBorrowsNew);
 

hardhat.config.js

@@ -0,0 +1,30 @@
+require("@nomiclabs/hardhat-waffle");
+
+/**
+ * @type import('hardhat/config').HardhatUserConfig
+ */
+module.exports = {
+  solidity: {
+    version: "0.5.17",
+    settings: {
+      optimizer: {
+        enabled: true,
+        runs: 200
+      }
+    }
+  },
+  networks: {
+    hardhat: {
+      forking: {
+        url: process.env.MAINNET_WEB3_PROVIDER,
+        blockNumber: process.env.FORK_BLOCK_NUMBER !== undefined ? parseInt(process.env.FORK_BLOCK_NUMBER) : undefined // Block before attack on pool 8 = 14684685
+      }
+    },
+    development: {
+      url: "http://localhost:8546"
+    }
+  },
+  paths: {
+    tests: "./hardhat/test"
+  }
+};

hardhat/scripts/fix-exploit.js

@@ -0,0 +1,71 @@
+// We require the Hardhat Runtime Environment explicitly here. This is optional
+// but useful for running the script in a standalone fashion through `node <script>`.
+//
+// When running the script with `npx hardhat run <script>` you'll find the Hardhat
+// Runtime Environment's members available in the global scope.
+const hre = require("hardhat");
+
+async function main() {
+  // Hardhat always runs the compile task when running scripts with its command
+  // line interface.
+  //
+  // If this script is run directly using `node` you may want to call compile
+  // manually to make sure everything is compiled
+  // await hre.run('compile');
+
+  // Enable using 0 gas price
+  await hre.network.provider.send("hardhat_setNextBlockBaseFeePerGas", ["0x0"]);
+
+  // Deploy new CEtherDelegateTempExploitAccounting
+  const CEtherDelegateTempExploitAccounting = await ethers.getContractFactory("CEtherDelegateTempExploitAccounting");
+  var cEtherDelegateTempExploitAccounting = await CEtherDelegateTempExploitAccounting.deploy();
+
+  // Deploy new CEtherDelegate
+  const CEtherDelegate = await ethers.getContractFactory("CEtherDelegate");
+  var cEtherDelegate = await CEtherDelegate.deploy();
+
+  // Get pool 8 admin
+  const Comptroller = await ethers.getContractFactory("Comptroller");
+  var comptroller = Comptroller.attach("0xc54172e34046c1653d1920d40333dd358c7a1af4");
+  var comptrollerAdmin = await comptroller.admin();
+
+  // Impersonate admin of pool 8
+  await hre.network.provider.request({
+      method: "hardhat_impersonateAccount",
+      params: [comptrollerAdmin],
+  });
+
+  // Get FuseFeeDistributor owner
+  var ffd = new ethers.Contract("0xa731585ab05fC9f83555cf9Bff8F58ee94e18F85", [{"inputs":[{"internalType":"address[]","name":"oldImplementations","type":"address[]"},{"internalType":"address[]","name":"newImplementations","type":"address[]"},{"internalType":"bool[]","name":"allowResign","type":"bool[]"},{"internalType":"bool[]","name":"statuses","type":"bool[]"}],"name":"_editCEtherDelegateWhitelist","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[],"name":"owner","outputs":[{"internalType":"address","name":"","type":"address"}],"stateMutability":"view","type":"function"}], ethers.provider);
+  var ffdOwner = await ffd.owner();
+
+  // Impersonate FuseFeeDistributor owner
+  await hre.network.provider.request({
+      method: "hardhat_impersonateAccount",
+      params: [ffdOwner],
+  });
+
+  // Call FuseFeeDistributor._editCEtherDelegateWhitelist
+  await ffd.connect(await ethers.getSigner(ffdOwner))._editCEtherDelegateWhitelist(["0xd77e28a1b9a9cfe1fc2eee70e391c05d25853cbf", cEtherDelegateTempExploitAccounting.address], [cEtherDelegateTempExploitAccounting.address, cEtherDelegate.address], [false, false], [true, true], { gasPrice: "0" });
+
+  // Call CEther._setImplementationSafe for temp impl
+  var cEther = CEtherDelegate.attach("0xbB025D470162CC5eA24daF7d4566064EE7f5F111");
+  var secondaryExploiterAddresses = ["0x3686657208883d016971c7395edaed73c107383e"];
+  var becomeImplData = ethers.utils.defaultAbiCoder.encode(["address[]"], [secondaryExploiterAddresses]);
+  await cEther.connect(await ethers.getSigner(comptrollerAdmin))._setImplementationSafe(cEtherDelegateTempExploitAccounting.address, false, becomeImplData, { gasPrice: "0" });
+
+  // Call CEther._setImplementationSafe for final impl
+  await cEther.connect(await ethers.getSigner(comptrollerAdmin))._setImplementationSafe(cEtherDelegate.address, false, "0x", { gasPrice: "0" });
+
+  // Call FuseFeeDistributor._editCEtherDelegateWhitelist again
+  await ffd.connect(await ethers.getSigner(ffdOwner))._editCEtherDelegateWhitelist(["0xd77e28a1b9a9cfe1fc2eee70e391c05d25853cbf", "0xd77e28a1b9a9cfe1fc2eee70e391c05d25853cbf"], [cEtherDelegateTempExploitAccounting.address, cEtherDelegate.address], [false, false], [false, true], { gasPrice: "0" });
+}
+
+// We recommend this pattern to be able to use async/await everywhere
+// and properly handle errors.
+main()
+  .then(() => process.exit(0))
+  .catch((error) => {
+    console.error(error);
+    process.exit(1);
+  });

hardhat/test/fix-accounting.js

@@ -0,0 +1,78 @@
+const { ethers } = require("hardhat");
+const { expect } = require("chai");
+
+describe("CEtherDelegateTempExploitAccounting", function () {
+  it("Should merge the attacker's supply and borrow balances", async function () {
+    // Enable using 0 gas price
+    await hre.network.provider.send("hardhat_setNextBlockBaseFeePerGas", ["0x0"]);
+
+    // Deploy new CEtherDelegateTempExploitAccounting
+    const CEtherDelegateTempExploitAccounting = await ethers.getContractFactory("CEtherDelegateTempExploitAccounting");
+    var cEtherDelegateTempExploitAccounting = await CEtherDelegateTempExploitAccounting.deploy();
+
+    // Deploy new CEtherDelegate
+    const CEtherDelegate = await ethers.getContractFactory("CEtherDelegate");
+    var cEtherDelegate = await CEtherDelegate.deploy();
+
+    // Get pool 8 admin
+    const Comptroller = await ethers.getContractFactory("Comptroller");
+    var comptroller = Comptroller.attach("0xc54172e34046c1653d1920d40333dd358c7a1af4");
+    var comptrollerAdmin = await comptroller.admin();
+
+    // Impersonate admin of pool 8
+    await hre.network.provider.request({
+        method: "hardhat_impersonateAccount",
+        params: [comptrollerAdmin],
+    });
+
+    // Get FuseFeeDistributor owner
+    var ffd = new ethers.Contract("0xa731585ab05fC9f83555cf9Bff8F58ee94e18F85", [{"inputs":[{"internalType":"address[]","name":"oldImplementations","type":"address[]"},{"internalType":"address[]","name":"newImplementations","type":"address[]"},{"internalType":"bool[]","name":"allowResign","type":"bool[]"},{"internalType":"bool[]","name":"statuses","type":"bool[]"}],"name":"_editCEtherDelegateWhitelist","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[],"name":"owner","outputs":[{"internalType":"address","name":"","type":"address"}],"stateMutability":"view","type":"function"}], ethers.provider);
+    var ffdOwner = await ffd.owner();
+
+    // Impersonate FuseFeeDistributor owner
+    await hre.network.provider.request({
+        method: "hardhat_impersonateAccount",
+        params: [ffdOwner],
+    });
+
+    // Call FuseFeeDistributor._editCEtherDelegateWhitelist
+    await ffd.connect(await ethers.getSigner(ffdOwner))._editCEtherDelegateWhitelist(["0xd77e28a1b9a9cfe1fc2eee70e391c05d25853cbf", cEtherDelegateTempExploitAccounting.address], [cEtherDelegateTempExploitAccounting.address, cEtherDelegate.address], [false, false], [true, true], { gasPrice: "0" });
+
+    // Get attacker's initial balances
+    var cEther = CEtherDelegate.attach("0xbB025D470162CC5eA24daF7d4566064EE7f5F111");
+    var exchangeRateStored = await cEther.exchangeRateStored();
+    var account1SupplySharesInitial = await cEther.balanceOf("0x32075bAd9050d4767018084F0Cb87b3182D36C45");
+    var account1UnderlyingSupplyBalanceInitial = account1SupplySharesInitial.mul(exchangeRateStored).div(ethers.utils.parseEther("1"));
+    var account1UnderlyingBorrowBalanceInitial = await cEther.borrowBalanceStored("0x32075bAd9050d4767018084F0Cb87b3182D36C45");
+    var account2UnderlyingSupplyBalanceInitial = (await cEther.balanceOf("0x3686657208883d016971c7395edaed73c107383e")).mul(exchangeRateStored).div(ethers.utils.parseEther("1"));
+    var account2UnderlyingBorrowBalanceInitial = await cEther.borrowBalanceStored("0x3686657208883d016971c7395edaed73c107383e");
+    expect(account1UnderlyingSupplyBalanceInitial).to.be.above(0);
+    expect(account1UnderlyingBorrowBalanceInitial).to.equal(0);
+    expect(account2UnderlyingSupplyBalanceInitial).to.equal(0);
+    expect(account2UnderlyingBorrowBalanceInitial).to.be.above(0);
+    var totalSupplyInitial = await cEther.totalSupply();
+    var totalBorrowsInitial = await cEther.totalBorrows();
+
+    // Call CEther._setImplementationSafe for temp impl
+    var secondaryExploiterAddresses = ["0x3686657208883d016971c7395edaed73c107383e"];
+    var becomeImplData = ethers.utils.defaultAbiCoder.encode(["address[]"], [secondaryExploiterAddresses]);
+    await cEther.connect(await ethers.getSigner(comptrollerAdmin))._setImplementationSafe(cEtherDelegateTempExploitAccounting.address, false, becomeImplData, { gasPrice: "0" });
+
+    // Call CEther._setImplementationSafe for final impl
+    await cEther.connect(await ethers.getSigner(comptrollerAdmin))._setImplementationSafe(cEtherDelegate.address, false, "0x", { gasPrice: "0" });
+
+    // Double-check attacker's balances
+    var account1UnderlyingSupplyBalanceFinal = (await cEther.balanceOf("0x32075bAd9050d4767018084F0Cb87b3182D36C45")).mul(exchangeRateStored);
+    var account1UnderlyingBorrowBalanceFinal = await cEther.borrowBalanceStored("0x32075bAd9050d4767018084F0Cb87b3182D36C45");
+    var account2UnderlyingSupplyBalanceFinal = (await cEther.balanceOf("0x3686657208883d016971c7395edaed73c107383e")).mul(exchangeRateStored);
+    var account2UnderlyingBorrowBalanceFinal = await cEther.borrowBalanceStored("0x3686657208883d016971c7395edaed73c107383e");
+    expect(account1UnderlyingSupplyBalanceFinal).to.equal(0);
+    expect(account1UnderlyingBorrowBalanceFinal).to.equal(account2UnderlyingBorrowBalanceInitial.sub(account1UnderlyingSupplyBalanceInitial));
+    expect(account2UnderlyingSupplyBalanceFinal).to.equal(0);
+    expect(account2UnderlyingBorrowBalanceFinal).to.equal(0);
+    var totalSupplyFinal = await cEther.totalSupply();
+    var totalBorrowsFinal = await cEther.totalBorrows();
+    expect(totalSupplyFinal).to.equal(totalSupplyInitial.sub(account1SupplySharesInitial));
+    expect(totalBorrowsFinal).to.equal(totalBorrowsInitial.sub(account1UnderlyingSupplyBalanceInitial));
+  });
+});