Fix bugs and add msg.sender and gaslimit check

contracts/interfaces/access/IAccessModule.sol

@@ -12,4 +12,6 @@ interface IAccessModule {
      * @param sender Sender of transaction
      */
     function isOperationAllowed(Operation operation, address sender) external view returns(bool);
+
+    function getMaxGasLeft(Operation operation) external view returns(uint256);
 }

contracts/modules/access/AccessChecker.sol

@@ -9,5 +9,9 @@ contract AccessChecker is Module {
         IAccessModule am = IAccessModule(getModuleAddress(MODULE_ACCESS));
         require(am.isOperationAllowed(operation, _msgSender()), "AccessChecker: operation not allowed");
         _;
+        uint256 maxGasLeft = am.getMaxGasLeft(operation);
+        if(maxGasLeft > 0) {
+            require(gasleft() <= maxGasLeft, "Too many gas left");
+        }
     }
 }

contracts/modules/access/AccessModule.sol

@@ -6,35 +6,49 @@ import "../../common/Module.sol";
 import "../../interfaces/access/IAccessModule.sol";
 
 contract AccessModule is Module, IAccessModule, Pausable, WhitelistedRole {
-    event WhitelistEnabled();
-    event WhitelistDisabled();
+    event WhitelistForAllStatusChange(bool enabled);
+    event WhitelistForIntermediateSendersStatusChange(bool enabled);
 
-    bool public whitelistEnabled;
+    bool public whitelistEnabledForAll;
+    bool public whitelistEnabledForIntermediateSenders;
+    mapping(uint8=>uint256) public maxGasLeft; //Zero value means no limit
 
     function initialize(address _pool) public initializer {
         Module.initialize(_pool);
         Pausable.initialize(_msgSender());
         WhitelistedRole.initialize(_msgSender());
-        // enableWhitelist(); //whitelist is disabled by default for testnet, will be enabled by default for mainnet
     }
 
-    function enableWhitelist() public onlyWhitelistAdmin {
-        whitelistEnabled = true;
-        emit WhitelistEnabled();
+    function setWhitelistForAll(bool enabled) public onlyWhitelistAdmin {
+        whitelistEnabledForAll = enabled;
+        emit WhitelistForAllStatusChange(enabled);
     }
 
-    function disableWhitelist() public onlyWhitelistAdmin {
-        whitelistEnabled = false;
-        emit WhitelistDisabled();
+    function setWhitelistForIntermediateSenders(bool enabled) public onlyWhitelistAdmin {
+        whitelistEnabledForIntermediateSenders = enabled;
+        emit WhitelistForIntermediateSendersStatusChange(enabled);
+    }
+
+    function setMaxGasLeft(Operation operation, uint256 value) public onlyWhitelistAdmin {
+        maxGasLeft[uint8(operation)] = value;
+    }
+
+    function getMaxGasLeft(Operation operation) public view returns(uint256) {
+        return maxGasLeft[uint8(operation)];
     }
 
     function isOperationAllowed(Operation operation, address sender) public view returns(bool) {
         (operation);    //noop to prevent compiler warning
         if (paused()) return false;
-        if (!whitelistEnabled) {
-            return true;
-        } else {
+        if (whitelistEnabledForAll) {
+            return isWhitelisted(sender);
+        } else if(
+            whitelistEnabledForIntermediateSenders && 
+            tx.origin != sender
+        ){
             return isWhitelisted(sender);
+        } else {
+            return true;
         }
     }
 }

contracts/modules/savings/SavingsModule.sol

@@ -58,12 +58,33 @@ contract SavingsModule is Module, AccessChecker, RewardDistributions, CapperRole
     mapping(address=>uint256) public protocolCap;
     bool public vipUserEnabled;                         // Enable VIP user (overrides protocol cap)
 
+    uint256 private _guardCounter; // See OpenZeppelin ReentrancyGuard. Copied here to allow upgrade of already deployed contracts    
+
 
     function initialize(address _pool) public initializer {
         Module.initialize(_pool);
         CapperRole.initialize(_msgSender());
+
+        _guardCounter = 1; // See OpenZeppelin ReentrancyGuard.
+    }
+
+    function upgradeGuardCounter() public onlyOwner {
+        require(_guardCounter == 0, "Already upgraded");
+        _guardCounter = 1;
+    }
+
+    /**
+     * @dev Prevents a contract from calling itself, directly or indirectly.
+     * See OpenZeppelin ReentrancyGuard
+     */
+    modifier nonReentrant() {
+        _guardCounter += 1;
+        uint256 localCounter = _guardCounter;
+        _;
+        require(localCounter == _guardCounter, "ReentrancyGuard: reentrant call");
     }
 
+
     function setUserCapEnabled(bool _userCapEnabled) public onlyCapper {
         userCapEnabled = _userCapEnabled;
         emit UserCapEnabledChange(userCapEnabled);
@@ -198,18 +219,20 @@ contract SavingsModule is Module, AccessChecker, RewardDistributions, CapperRole
      * @param _tokens Array of tokens to deposit
      * @param _dnAmounts Array of amounts (denormalized to token decimals)
      */
-    function deposit(address[] memory _protocols, address[] memory _tokens, uint256[] memory _dnAmounts) 
-    public operationAllowed(IAccessModule.Operation.Deposit) 
+    function deposit(address[] calldata _protocols, address[] calldata _tokens, uint256[] calldata _dnAmounts) 
+    external nonReentrant operationAllowed(IAccessModule.Operation.Deposit) 
     returns(uint256[] memory) 
     {
         require(_protocols.length == _tokens.length && _tokens.length == _dnAmounts.length, "SavingsModule: size of arrays does not match");
+        require(isAllTokensRegistered(_tokens), "SavingsModule: unsupported token");
+
         uint256[] memory ptAmounts = new uint256[](_protocols.length);
         for (uint256 i=0; i < _protocols.length; i++) {
             address[] memory tkns = new address[](1);
             tkns[0] = _tokens[i];
             uint256[] memory amnts = new uint256[](1);
             amnts[0] = _dnAmounts[i];
-            ptAmounts[i] = deposit(_protocols[i], tkns, amnts);
+            ptAmounts[i] = _deposit(_protocols[i], tkns, amnts);
         }
         return ptAmounts;
     }
@@ -220,8 +243,15 @@ contract SavingsModule is Module, AccessChecker, RewardDistributions, CapperRole
      * @param _tokens Array of tokens to deposit
      * @param _dnAmounts Array of amounts (denormalized to token decimals)
      */
-    function deposit(address _protocol, address[] memory _tokens, uint256[] memory _dnAmounts)
-    public operationAllowed(IAccessModule.Operation.Deposit)
+    function deposit(address _protocol, address[] calldata _tokens, uint256[] calldata _dnAmounts)
+    external nonReentrant operationAllowed(IAccessModule.Operation.Deposit)
+    returns(uint256) {
+        require(isAllTokensRegistered(_tokens), "SavingsModule: unsupported token");
+        _deposit(_protocol, _tokens, _dnAmounts);
+    }
+
+    function _deposit(address _protocol, address[] memory _tokens, uint256[] memory _dnAmounts)
+    internal
     returns(uint256) 
     {
         //distributeRewardIfRequired(_protocol);
@@ -293,7 +323,7 @@ contract SavingsModule is Module, AccessChecker, RewardDistributions, CapperRole
      * @return Amount of PoolToken burned from user
      */
     function withdrawAll(address _protocol, uint256 nAmount)
-    public operationAllowed(IAccessModule.Operation.Withdraw)
+    external nonReentrant operationAllowed(IAccessModule.Operation.Withdraw)
     returns(uint256) 
     {
         //distributeRewardIfRequired(_protocol);
@@ -341,8 +371,9 @@ contract SavingsModule is Module, AccessChecker, RewardDistributions, CapperRole
      * @return Amount of PoolToken burned from user
      */
     function withdraw(address _protocol, address token, uint256 dnAmount, uint256 maxNAmount)
-    public operationAllowed(IAccessModule.Operation.Withdraw)
+    external nonReentrant operationAllowed(IAccessModule.Operation.Withdraw)
     returns(uint256){
+        require(isTokenRegistered(token), "SavingsModule: unsupported token");
         //distributeRewardIfRequired(_protocol);
 
         uint256 nAmount = normalizeTokenAmount(token, dnAmount);
@@ -353,13 +384,13 @@ contract SavingsModule is Module, AccessChecker, RewardDistributions, CapperRole
 
         uint256 yield;
         uint256 actualAmount;
-        uint256 fee;
+        //uint256 fee;
         if(nBalanceAfter.add(nAmount) > nBalanceBefore) {
             yield = nBalanceAfter.add(nAmount).sub(nBalanceBefore);
             actualAmount = nAmount;
         }else{
             actualAmount = nBalanceBefore.sub(nBalanceAfter);
-            if (actualAmount > nAmount) fee = actualAmount-nAmount;
+            //if (actualAmount > nAmount) fee = actualAmount-nAmount;
         }
 
         require(maxNAmount == 0 || actualAmount <= maxNAmount, "SavingsModule: provided maxNAmount is too low");
@@ -374,7 +405,7 @@ contract SavingsModule is Module, AccessChecker, RewardDistributions, CapperRole
         PoolToken poolToken = PoolToken(protocols[_protocol].poolToken);
         poolToken.burnFrom(_msgSender(), actualAmount);
         emit WithdrawToken(_protocol, token, dnAmount);
-        emit Withdraw(_protocol, _msgSender(), actualAmount, fee);
+        emit Withdraw(_protocol, _msgSender(), actualAmount, /*fee*/ (actualAmount>nAmount)?actualAmount.sub(nAmount):0 );
 
 
         if (yield > 0) {
@@ -388,7 +419,7 @@ contract SavingsModule is Module, AccessChecker, RewardDistributions, CapperRole
     /** 
      * @notice Distributes yield. May be called by bot, if there was no deposits/withdrawals
      */
-    function distributeYield() public {
+    function distributeYield() external {
         for(uint256 i=0; i<registeredProtocols.length; i++) {
             distributeYieldInternal(address(registeredProtocols[i]));
         }
@@ -496,6 +527,13 @@ contract SavingsModule is Module, AccessChecker, RewardDistributions, CapperRole
         return false;
     }
 
+    function isAllTokensRegistered(address[] memory _tokens) private view returns(bool) {
+        for (uint256 i = 0; i < _tokens.length; i++){
+            if(!isTokenRegistered(_tokens[i])) return false;
+        }
+        return true;
+    }
+
     function isPoolToken(address token) internal view returns(bool) {
         for (uint256 i = 0; i < registeredProtocols.length; i++){
             IDefiProtocol protocol = registeredProtocols[i];

crytic-config.json

@@ -0,0 +1,20 @@
+{
+    "cwd": "",
+    "node_version": "12.19.0",
+    "solc": "0.5.17",
+    "use_yarn": true,
+    "custom_unit_test": {
+        "enabled": true,
+        "commands": [
+            {
+                "command": ["yarn", "test"]
+            }
+        ]
+    },
+    "slither": {
+        "target": "."
+    },
+    "echidna": {
+        "contract": ""
+    }
+}