Make Circle CI fail if Slither finds a high impact detector (trusttoken#682)

* Rename requirements.txt to requirements-dev.txt

* Make CI fail on slither

* Exclude optimization and low impact detectors

* Make slither script not stop at first non-zero status

* Fix slither detectors2 (trusttoken#731)

* Ignore slither warnings

* Do not ignore reentrancy vulnerabilities

* Exclude all but high impact detectors

* Remove unnecessary slither annotations

* split steps

Co-authored-by: Anze1m <bartek@ethworks.io>

Author: skalermo

.circleci/config.yml

@@ -11,18 +11,18 @@ commands:
       - restore_cache:
           name: Restore modules cache
           keys:
-            - node_modules-{{ checksum "yarn.lock" }}-{{ checksum "requirements.txt" }}
+            - node_modules-{{ checksum "yarn.lock" }}-{{ checksum "requirements-dev.txt" }}
       - run:
           name: Install Dependencies
           command: |
             yarn install --frozen-lockfile
             pip3 install -U pip setuptools virtualenv
             python3 -m venv venv
             source venv/bin/activate
-            pip3 install -r requirements.txt
+            pip3 install -r requirements-dev.txt
       - save_cache:
           name: Save modules cache
-          key: node_modules-{{ checksum "yarn.lock" }}-{{ checksum "requirements.txt" }}
+          key: node_modules-{{ checksum "yarn.lock" }}-{{ checksum "requirements-dev.txt" }}
           paths:
             - ./node_modules
             - ./venv
@@ -72,7 +72,13 @@ jobs:
     steps:
       - attach_workspace:
           at: .
-      - run: yarn test:governance && yarn test:proxy && yarn test:registry && yarn test:scripts && yarn test:true-currencies && yarn test:true-gold && yarn test:trusttoken
+      - run: yarn test:governance
+      - run: yarn test:proxy
+      - run: yarn test:registry
+      - run: yarn test:scripts
+      - run: yarn test:true-currencies
+      - run: yarn test:true-gold
+      - run: yarn test:trusttoken
   test-truefi:
     docker:
       - image: cimg/node:16.1.0

contracts/governance/GovernorAlpha.sol

@@ -252,6 +252,7 @@ contract GovernorAlpha is UpgradeableClaimable {
         proposal.executed = true;
         for (uint i = 0; i < proposal.targets.length; i++) {
             //OLD: timelock.executeTransaction.value(proposal.values[i])(proposal.targets[i], proposal.values[i], proposal.signatures[i], proposal.calldatas[i], proposal.eta);
+            // slither-disable-next-line arbitrary-send
             timelock.executeTransaction{value: proposal.values[i]}(proposal.targets[i], proposal.values[i], proposal.signatures[i], proposal.calldatas[i], proposal.eta);
         }
         emit ProposalExecuted(proposalId);

contracts/governance/StkTruToken.sol

@@ -272,6 +272,7 @@ contract StkTruToken is VoteToken, StkClaimableContract, IPauseableContract, Ree
      * Claims rewards when unstaking
      * @param amount Amount of stkTRU to unstake for TRU
      */
+    // slither-disable-next-line reentrancy-eth
     function unstake(uint256 amount) external distribute update(msg.sender) nonReentrant {
         require(amount > 0, "StkTruToken: Cannot unstake 0");
 
@@ -344,6 +345,7 @@ contract StkTruToken is VoteToken, StkClaimableContract, IPauseableContract, Ree
     /**
      * @dev Claim all rewards
      */
+    // slither-disable-next-line reentrancy-eth
     function claim() external distribute update(msg.sender) {
         _claim(tru);
         _claim(tfusd);

contracts/truefi/TrueLender.sol

@@ -430,7 +430,9 @@ contract TrueLender is ITrueLender, Ownable {
         uint256 denominator
     ) external override onlyPool {
         for (uint256 index = 0; index < _loans.length; index++) {
-            _loans[index].transfer(recipient, numerator.mul(_loans[index].balanceOf(address(this))).div(denominator));
+            if (!_loans[index].transfer(recipient, numerator.mul(_loans[index].balanceOf(address(this))).div(denominator))) {
+                0;
+            }
         }
     }
 

contracts/truefi/TrueRatingAgencyV2.sol

@@ -377,6 +377,7 @@ contract TrueRatingAgencyV2 is ITrueRatingAgencyV2, Ownable {
      * R = Total Reward = (interest * chi * rewardFactor)
      * @param id Loan ID
      */
+    // slither-disable-next-line reentrancy-no-eth
     modifier calculateTotalReward(address id) {
         if (loans[id].reward == 0) {
             uint256 interest = ILoanToken2(id).profit();
@@ -401,7 +402,10 @@ contract TrueRatingAgencyV2 is ITrueRatingAgencyV2, Ownable {
             loans[id].reward = ratersReward;
             if (totalReward > 0) {
                 distributor.distribute(totalReward);
-                TRU.transfer(address(stkTRU), totalReward.sub(ratersReward));
+                if (!TRU.transfer(address(stkTRU), totalReward.sub(ratersReward))) {
+                    // handle transfer failure
+                    0;
+                }
             }
         }
         _;

requirements.txt requirements-dev.txtrequirements.txt renamed to requirements-dev.txt

@@ -1,6 +1,8 @@
 {
+    "exclude_optimization": true,
     "exclude_informational": true,
-    "exclude_low": false,
+    "exclude_low": true,
+    "exclude_medium": true,
     "solc_disable_warnings": true,
     "detectors_to_exclude": ""
-}
+}

slither.config.json

@@ -23,14 +23,15 @@ solc-select use 0.6.10
 
 yarn flatten
 
-slither flatten/GovernorAlpha.sol --print human-summary
-slither flatten/Liquidator.sol --print human-summary
-slither flatten/LoanFactory.sol --print human-summary
-slither flatten/LoanToken.sol --print human-summary
-slither flatten/StkTruToken.sol --print human-summary
-slither flatten/TrueLender.sol --print human-summary
-slither flatten/TrueRatingAgencyV2.sol --print human-summary
-slither flatten/TrustToken.sol --print human-summary
-slither flatten/Timelock.sol --print human-summary
+status=0
+slither flatten/GovernorAlpha.sol || status=1
+slither flatten/Liquidator.sol || status=1
+slither flatten/LoanFactory.sol || status=1
+slither flatten/LoanToken.sol || status=1
+slither flatten/StkTruToken.sol || status=1
+slither flatten/TrueLender.sol || status=1
+slither flatten/TrueRatingAgencyV2.sol || status=1
+slither flatten/TrustToken.sol || status=1
+slither flatten/Timelock.sol || status=1
 
-echo "Done."
+exit $status