some more basic security fixes

contracts/CompoundAllocationStrategy.sol

@@ -1,10 +1,11 @@
 pragma solidity ^0.5.8;
 
 import {IAllocationStrategy} from "./IAllocationStrategy.sol";
+import {Ownable} from "openzeppelin-solidity/contracts/ownership/Ownable.sol";
 import {IERC20} from "openzeppelin-solidity/contracts/token/ERC20/IERC20.sol";
 import {CErc20Interface} from "../compound/contracts/CErc20Interface.sol";
 
-contract CompoundAllocationStrategy is IAllocationStrategy {
+contract CompoundAllocationStrategy is IAllocationStrategy, Ownable {
 
     CErc20Interface cToken;
     IERC20 token;
@@ -30,7 +31,7 @@ contract CompoundAllocationStrategy is IAllocationStrategy {
     }
 
     /// @dev ISavingStrategy.investUnderlying implementation
-    function investUnderlying(uint256 investAmount) external returns (uint256) {
+    function investUnderlying(uint256 investAmount) external onlyOwner returns (uint256) {
         token.transferFrom(msg.sender, address(this), investAmount);
         token.approve(address(cToken), investAmount);
         uint256 cTotalBefore = cToken.totalSupply();
@@ -45,7 +46,7 @@ contract CompoundAllocationStrategy is IAllocationStrategy {
     }
 
     /// @dev ISavingStrategy.redeemUnderlying implementation
-    function redeemUnderlying(uint256 redeemAmount) external returns (uint256) {
+    function redeemUnderlying(uint256 redeemAmount) external onlyOwner returns (uint256) {
         uint256 cTotalBefore = cToken.totalSupply();
         // TODO should we handle redeem failure?
         require(cToken.redeemUnderlying(redeemAmount) == 0, "redeemUnderlying failed");

package.json

@@ -9,7 +9,7 @@
         "truffle-security": "^1.5.3"
     },
     "devDependencies": {
-        "@decentral.ee/web3-test-helpers": "^0.1.0",
+        "@decentral.ee/web3-test-helpers": "^0.2.0",
         "dotenv": "^8.0.0",
         "openzeppelin-test-helpers": "^0.4.2",
         "truffle": "^5.0.31",

scripts/deploy-rdai.js

@@ -34,6 +34,8 @@ module.exports = async (callback) => {
                 }
             );
             console.log("rDai deployed at: ", rToken.address);
+            console.log("transfer ownership of compoundAS to new rDai", rToken.address);
+            await web3tx(compoundAS.transferOwnership, "compoundAS.transferOwnership")(rToken.address);
         }
         callback();
     } catch (err) {

test/RToken.test.js

@@ -5,16 +5,7 @@ const InterestRateModelMock = artifacts.require("InterestRateModelMock");
 const CErc20 = artifacts.require("CErc20");
 const CompoundAllocationStrategy = artifacts.require("CompoundAllocationStrategy");
 const RToken = artifacts.require("RToken");
-const { web3tx } = require("@decentral.ee/web3-test-helpers");
-const { toDecimals, fromDecimals } = require("../lib/math-utils");
-
-function wad4human(wad, decimals = 5) {
-    return Number(fromDecimals(wad.toString(), 18)).toFixed(decimals);
-}
-
-function toWad(n) {
-    return web3.utils.toBN(toDecimals(n, 18));
-}
+const { web3tx, wad4human, toWad } = require("@decentral.ee/web3-test-helpers");
 
 contract("RToken contract", accounts => {
     //const ZERO_ADDRESS = "0x0000000000000000000000000000000000000000";
@@ -27,6 +18,7 @@ contract("RToken contract", accounts => {
     const customer4 = accounts[5];
     let token;
     let cToken;
+    let compoundAS;
     let rToken;
     let SELF_HAT_ID;
 
@@ -53,15 +45,16 @@ contract("RToken contract", accounts => {
             18, {
                 from: admin
             });
-        const compoundSS = await web3tx(CompoundAllocationStrategy.new, "CompoundAllocationStrategy.new")(
+        compoundAS = await web3tx(CompoundAllocationStrategy.new, "CompoundAllocationStrategy.new")(
             cToken.address, {
                 from: admin
             }
         );
         rToken = await web3tx(RToken.new, "RToken.new")(
-            compoundSS.address, {
+            compoundAS.address, {
                 from: admin
             });
+        await web3tx(compoundAS.transferOwnership, "compoundAS.transferOwnership")(rToken.address);
         SELF_HAT_ID = await rToken.SELF_HAT_ID.call();
     });
 
@@ -967,4 +960,21 @@ contract("RToken contract", accounts => {
             interestPayable: "0.00000",
         });
     });
+
+    it("#10 CompoundAs ownership protection", async () => {
+        await web3tx(token.approve, "token.approve 100 by customer1")(rToken.address, toWad(100), {
+            from: customer1
+        });
+        await expectRevert(rToken.mintWithSelectedHat(toWad(1), 1), "Invalid hat ID");
+        await web3tx(rToken.mintWithSelectedHat, "rToken.mintWithSelectedHat 100 to customer1 with the self hat", {
+            inLogs: [{
+                name: "Mint"
+            }]
+        })(toWad(100), await rToken.SELF_HAT_ID.call(), {
+            from: customer1
+        });
+        await expectRevert(web3tx(compoundAS.redeemUnderlying, "redeemUnderlying by admin")(toWad(100), {
+            from: admin
+        }), "Ownable: caller is not the owner");
+    });
 });