Uploaded files for judging

001.md

@@ -0,0 +1,134 @@
+Sunny Latte Trout
+
+Medium
+
+# Missing deadline check in swap functions
+
+### Description
+
+Swap functions don't have deadline parameter. This parameter can provide the user an option to limit the execution of their pending transaction.
+Without a deadline parameter, users can execute their transactions at unexpected times when market conditions are unfavorable.
+
+However, this is not a big problem in this case because the functions have slippage protection. Even though the users will get at least as much as they set, they may still be missing out on positive slippage if the exchange rate becomes favorable when the transaction is included in a block.
+
+### Code Snippets
+
+https://github.com/sherlock-audit/2024-10-mento-update/blob/098b17fb32d294145a7f000d96917d13db8756cc/mento-core/contracts/swap/Broker.sol#L145-L184
+
+```solidity
+  function swapIn(
+    address exchangeProvider,
+    bytes32 exchangeId,
+    address tokenIn,
+    address tokenOut,
+    uint256 amountIn,
+    uint256 amountOutMin
+  ) external nonReentrant returns (uint256 amountOut) {
+    require(isExchangeProvider[exchangeProvider], "ExchangeProvider does not exist");
+    // slither-disable-next-line reentrancy-benign
+    amountOut = IExchangeProvider(exchangeProvider).swapIn(exchangeId, tokenIn, tokenOut, amountIn);
+    require(amountOut >= amountOutMin, "amountOutMin not met");
+    guardTradingLimits(exchangeId, tokenIn, amountIn, tokenOut, amountOut);
+
+
+    address reserve = exchangeReserve[exchangeProvider];
+    transferIn(payable(msg.sender), tokenIn, amountIn, reserve);
+    transferOut(payable(msg.sender), tokenOut, amountOut, reserve);
+    emit Swap(exchangeProvider, exchangeId, msg.sender, tokenIn, tokenOut, amountIn, amountOut);
+  }
+
+
+  /// @inheritdoc IBroker
+  function swapOut(
+    address exchangeProvider,
+    bytes32 exchangeId,
+    address tokenIn,
+    address tokenOut,
+    uint256 amountOut,
+    uint256 amountInMax
+  ) external nonReentrant returns (uint256 amountIn) {
+    require(isExchangeProvider[exchangeProvider], "ExchangeProvider does not exist");
+    // slither-disable-next-line reentrancy-benign
+    amountIn = IExchangeProvider(exchangeProvider).swapOut(exchangeId, tokenIn, tokenOut, amountOut);
+    require(amountIn <= amountInMax, "amountInMax exceeded");
+    guardTradingLimits(exchangeId, tokenIn, amountIn, tokenOut, amountOut);
+
+
+    address reserve = exchangeReserve[exchangeProvider];
+    transferIn(payable(msg.sender), tokenIn, amountIn, reserve);
+    transferOut(payable(msg.sender), tokenOut, amountOut, reserve);
+    emit Swap(exchangeProvider, exchangeId, msg.sender, tokenIn, tokenOut, amountIn, amountOut);
+  }
+```
+
+https://github.com/sherlock-audit/2024-10-mento-update/blob/098b17fb32d294145a7f000d96917d13db8756cc/mento-core/contracts/goodDollar/BancorExchangeProvider.sol#L191-L221
+
+```solidity
+  /// @inheritdoc IExchangeProvider
+  function swapIn(
+    bytes32 exchangeId,
+    address tokenIn,
+    address tokenOut,
+    uint256 amountIn
+  ) public virtual onlyBroker returns (uint256 amountOut) {
+    PoolExchange memory exchange = getPoolExchange(exchangeId);
+    uint256 scaledAmountIn = amountIn * tokenPrecisionMultipliers[tokenIn];
+    uint256 scaledAmountOut = _getScaledAmountOut(exchange, tokenIn, tokenOut, scaledAmountIn);
+    executeSwap(exchangeId, tokenIn, scaledAmountIn, scaledAmountOut);
+
+
+    amountOut = scaledAmountOut / tokenPrecisionMultipliers[tokenOut];
+    return amountOut;
+  }
+
+
+  /// @inheritdoc IExchangeProvider
+  function swapOut(
+    bytes32 exchangeId,
+    address tokenIn,
+    address tokenOut,
+    uint256 amountOut
+  ) public virtual onlyBroker returns (uint256 amountIn) {
+    PoolExchange memory exchange = getPoolExchange(exchangeId);
+    uint256 scaledAmountOut = amountOut * tokenPrecisionMultipliers[tokenOut];
+    uint256 scaledAmountIn = _getScaledAmountIn(exchange, tokenIn, tokenOut, scaledAmountOut);
+    executeSwap(exchangeId, tokenIn, scaledAmountIn, scaledAmountOut);
+
+
+    amountIn = scaledAmountIn / tokenPrecisionMultipliers[tokenIn];
+    return amountIn;
+  }
+```
+
+https://github.com/sherlock-audit/2024-10-mento-update/blob/098b17fb32d294145a7f000d96917d13db8756cc/mento-core/contracts/goodDollar/GoodDollarExchangeProvider.sol#L113-L131
+
+```solidity
+  /// @inheritdoc BancorExchangeProvider
+  function swapIn(
+    bytes32 exchangeId,
+    address tokenIn,
+    address tokenOut,
+    uint256 amountIn
+  ) public override onlyBroker whenNotPaused returns (uint256 amountOut) {
+    amountOut = BancorExchangeProvider.swapIn(exchangeId, tokenIn, tokenOut, amountIn);
+  }
+
+
+  /// @inheritdoc BancorExchangeProvider
+  function swapOut(
+    bytes32 exchangeId,
+    address tokenIn,
+    address tokenOut,
+    uint256 amountOut
+  ) public override onlyBroker whenNotPaused returns (uint256 amountIn) {
+    amountIn = BancorExchangeProvider.swapOut(exchangeId, tokenIn, tokenOut, amountOut);
+  }
+```
+
+### Recommendations
+
+Introduce a `deadline` parameter in all swap functions.
+
+### Reference
+
+https://github.com/pashov/audits/blob/master/team/md/Kekotron-security-review.md#m-03-missing-deadline-check-in-swap-functions

001/028.md

@@ -0,0 +1,182 @@
+Pet Tawny Loris
+
+Medium
+
+# Malicious users can immediately reduce the original reserve ratio after the exchange is created.
+
+### Summary
+
+When an exchange is created, the reserve ratio is provided as an initial ratio. In my opinion, the exchange owner might want to maintain that reserve ratio for at least one extension step(e.g. 1 day).
+
+However, by back-running the exchange creation, the given reserve ratio can be shrunk by an extension rate.
+
+### Root Cause
+
+The [`GoodDollarExpansionController::_getReserveRatioScalar()`](https://github.com/sherlock-audit/2024-10-mento-update/blob/main/mento-core/contracts/goodDollar/GoodDollarExpansionController.sol#L225-L237) function calculates the number of extension steps to be applied to the reserve ratio of an exchange.
+
+```solidity
+  function _getReserveRatioScalar(ExchangeExpansionConfig memory config) internal view returns (uint256) {
+    uint256 numberOfExpansions;
+
+    // If there was no previous expansion, we expand once.
+    if (config.lastExpansion == 0) {
+      numberOfExpansions = 1;
+    } else {
+      numberOfExpansions = (block.timestamp - config.lastExpansion) / config.expansionFrequency;
+    }
+
+    uint256 stepReserveRatioScalar = MAX_WEIGHT - config.expansionRate;
+    return unwrap(powu(wrap(stepReserveRatioScalar), numberOfExpansions));
+  }
+```
+
+Upon creation of an exchange and its expansion configuration, the `lastExpansion` value is initialized to `0`. When the `mintUBIFromExpansion()` function is invoked for the first time, a single expansion is applied to the exchange.
+
+A malicious user can exploit this issue to immediately shrink the original reserve ratio by back-running an exchange creation.
+
+### Internal pre-conditions
+
+_No response_
+
+### External pre-conditions
+
+_No response_
+
+### Attack Path
+
+The attack scenario is quite simple:
+
+1. A malicious actor waits for the creation of any new exchange.
+2. Right after an exchange is created, the actor invokes the `GoodDollarExpansionController::mintUBIFromExpansion()` function.
+
+### Impact
+
+The owner's intended reserve ratio cannot be applied.
+
+### PoC
+
+Here is the test case to demonstrate the back-running attack as soon as new exchange has been created:
+
+```solidity
+// SPDX-License-Identifier: GPL-3.0-or-later
+pragma solidity 0.8.18;
+// solhint-disable func-name-mixedcase, var-name-mixedcase, state-visibility
+// solhint-disable const-name-snakecase, max-states-count, contract-name-camelcase
+
+import {console} from "forge-std/console.sol";
+
+import { Test } from "forge-std/Test.sol";
+import { ERC20Mock } from "openzeppelin-contracts-next/contracts/mocks/ERC20Mock.sol";
+import { GoodDollarExchangeProvider } from "contracts/goodDollar/GoodDollarExchangeProvider.sol";
+import { GoodDollarExpansionController } from "contracts/goodDollar/GoodDollarExpansionController.sol";
+
+import { IGoodDollarExpansionController } from "contracts/interfaces/IGoodDollarExpansionController.sol";
+import { IGoodDollarExchangeProvider } from "contracts/interfaces/IGoodDollarExchangeProvider.sol";
+import { IBancorExchangeProvider } from "contracts/interfaces/IBancorExchangeProvider.sol";
+import { IDistributionHelper } from "contracts/goodDollar/interfaces/IGoodProtocol.sol";
+import { IReserve } from "contracts/interfaces/IReserve.sol";
+
+import { GoodDollarExpansionControllerHarness } from "test/utils/harnesses/GoodDollarExpansionControllerHarness.sol";
+
+import { GoodDollarExpansionControllerTest } from "../goodDollar/GoodDollarExpansionController.t.sol";
+
+contract ExpansionControllerExpansionBackRunningTest is GoodDollarExpansionControllerTest {
+  GoodDollarExpansionController expansionController;
+  address brokerAddress;
+
+  function setUp() public override {
+    super.setUp();
+
+    brokerAddress = makeAddr("Broker");
+    
+    vm.mockCall(
+      reserveAddress,
+      abi.encodeWithSelector(IReserve(reserveAddress).isStableAsset.selector, address(token)),
+      abi.encode(true)
+    );
+    vm.mockCall(
+      reserveAddress,
+      abi.encodeWithSelector(IReserve(reserveAddress).isCollateralAsset.selector, address(reserveToken)),
+      abi.encode(true)
+    );
+    vm.mockCall(
+      distributionHelper,
+      abi.encodeWithSelector(IDistributionHelper(distributionHelper).onDistribution.selector),
+      abi.encode(true)
+    );
+
+    expansionController = initializeGoodDollarExpansionController();
+
+    exchangeProvider = address(new GoodDollarExchangeProvider(false));
+    IGoodDollarExchangeProvider(exchangeProvider).initialize(brokerAddress, reserveAddress, address(expansionController), avatarAddress);
+
+    expansionController.setGoodDollarExchangeProvider(exchangeProvider);
+
+    vm.startPrank(avatarAddress);
+    exchangeId = IBancorExchangeProvider(exchangeProvider).createExchange(pool);
+    expansionController.setExpansionConfig(exchangeId, expansionRate, expansionFrequency);
+    console.log("Exchange creation timestamp:", block.timestamp);
+    vm.stopPrank();
+  }
+
+  function test_mintUBIFromExpansion_backrunningExchangeCreation() public {
+    // @audit: At this moment, the exchange has just been created and then its configuration has been set
+    IGoodDollarExpansionController.ExchangeExpansionConfig memory config = expansionController.getExpansionConfig(
+      exchangeId
+    );
+    assert(block.timestamp < config.lastExpansion + config.expansionFrequency); // Ensure no expansion step is passed
+    IBancorExchangeProvider.PoolExchange memory poolExchange = IBancorExchangeProvider(exchangeProvider).getPoolExchange(exchangeId);
+    console.log("Reserve Ratio before being attacked:", poolExchange.reserveRatio);
+
+    console.log("Minting from expansion timestamp:", block.timestamp);
+    uint256 amountMinted = expansionController.mintUBIFromExpansion(exchangeId);
+
+    poolExchange = IBancorExchangeProvider(exchangeProvider).getPoolExchange(exchangeId);
+    console.log("Reserve Ratio after being attacked:", poolExchange.reserveRatio);
+  }
+}
+```
+
+Output:
+```log
+[PASS] test_mintUBIFromExpansion_backrunningExchangeCreation() (gas: 117674)
+Logs:
+  Exchange creation timestamp: 1
+  Reserve Ratio before being attacked: 20000000
+  Minting from expansion timestamp: 1
+  Reserve Ratio after being attacked: 19800000
+```
+
+As can be seen from the logs, the attack is able to shrink original reserve ratio without any delay.
+
+### Mitigation
+
+Two alternative mitigation options:
+
+1. Setting `lastExpansion` when the expansion configuration is set
+
+```diff
+  function setExpansionConfig(bytes32 exchangeId, uint64 expansionRate, uint32 expansionFrequency) external onlyAvatar {
+    ... ...
+
+    exchangeExpansionConfigs[exchangeId].expansionRate = expansionRate;
+    exchangeExpansionConfigs[exchangeId].expansionFrequency = expansionFrequency;
++   exchangeExpansionConfigs[exchangeId].lastExpansion = block.timestamp;
+
+    emit ExpansionConfigSet(exchangeId, expansionRate, expansionFrequency);
+  }
+```
+
+2. Disallowing an expansion for the very first attempt
+```diff
+  function _getReserveRatioScalar(ExchangeExpansionConfig memory config) internal view returns (uint256) {
+    ... ...
+    if (config.lastExpansion == 0) {
+-     numberOfExpansions = 1;
++     numberOfExpansions = 0;
+    } else {
+      numberOfExpansions = (block.timestamp - config.lastExpansion) / config.expansionFrequency;
+    }
+    ... ...
+  }
+```