Merge pull request #630 from morpho-org/fix/cantina-13

various fixes

src/Morpho.sol

@@ -401,13 +401,13 @@ contract Morpho is IMorphoStaticTyping {
             position[id][borrower].borrowShares = 0;
         }
 
-        IERC20(marketParams.collateralToken).safeTransfer(msg.sender, seizedAssets);
-
         // `repaidAssets` may be greater than `totalBorrowAssets` by 1.
         emit EventsLib.Liquidate(
             id, msg.sender, borrower, repaidAssets, repaidShares, seizedAssets, badDebtAssets, badDebtShares
         );
 
+        IERC20(marketParams.collateralToken).safeTransfer(msg.sender, seizedAssets);
+
         if (data.length > 0) IMorphoLiquidateCallback(msg.sender).onMorphoLiquidate(repaidAssets, data);
 
         IERC20(marketParams.loanToken).safeTransferFrom(msg.sender, address(this), repaidAssets);
@@ -419,10 +419,12 @@ contract Morpho is IMorphoStaticTyping {
 
     /// @inheritdoc IMorphoBase
     function flashLoan(address token, uint256 assets, bytes calldata data) external {
-        IERC20(token).safeTransfer(msg.sender, assets);
+        require(assets != 0, ErrorsLib.ZERO_ASSETS);
 
         emit EventsLib.FlashLoan(msg.sender, token, assets);
 
+        IERC20(token).safeTransfer(msg.sender, assets);
+
         IMorphoFlashLoanCallback(msg.sender).onMorphoFlashLoan(assets, data);
 
         IERC20(token).safeTransferFrom(msg.sender, address(this), assets);
@@ -432,18 +434,24 @@ contract Morpho is IMorphoStaticTyping {
 
     /// @inheritdoc IMorphoBase
     function setAuthorization(address authorized, bool newIsAuthorized) external {
+        require(newIsAuthorized != isAuthorized[msg.sender][authorized], ErrorsLib.ALREADY_SET);
+
         isAuthorized[msg.sender][authorized] = newIsAuthorized;
 
         emit EventsLib.SetAuthorization(msg.sender, msg.sender, authorized, newIsAuthorized);
     }
 
     /// @inheritdoc IMorphoBase
     function setAuthorizationWithSig(Authorization memory authorization, Signature calldata signature) external {
+        require(
+            authorization.isAuthorized != isAuthorized[authorization.authorizer][authorization.authorized],
+            ErrorsLib.ALREADY_SET
+        );
         require(block.timestamp <= authorization.deadline, ErrorsLib.SIGNATURE_EXPIRED);
         require(authorization.nonce == nonce[authorization.authorizer]++, ErrorsLib.INVALID_NONCE);
 
         bytes32 hashStruct = keccak256(abi.encode(AUTHORIZATION_TYPEHASH, authorization));
-        bytes32 digest = keccak256(abi.encodePacked("\x19\x01", DOMAIN_SEPARATOR, hashStruct));
+        bytes32 digest = keccak256(bytes.concat("\x19\x01", DOMAIN_SEPARATOR, hashStruct));
         address signatory = ecrecover(digest, signature.v, signature.r, signature.s);
 
         require(signatory != address(0) && authorization.authorizer == signatory, ErrorsLib.INVALID_SIGNATURE);

src/interfaces/IIrm.sol

@@ -8,11 +8,12 @@ import {MarketParams, Market} from "./IMorpho.sol";
 /// @custom:contact [email protected]
 /// @notice Interface that Interest Rate Models (IRMs) used by Morpho must implement.
 interface IIrm {
-    /// @notice Returns the borrow rate of the market `marketParams`.
+    /// @notice Returns the borrow rate per second (scaled by WAD) of the market `marketParams`.
     /// @dev Assumes that `market` corresponds to `marketParams`.
     function borrowRate(MarketParams memory marketParams, Market memory market) external returns (uint256);
 
-    /// @notice Returns the borrow rate of the market `marketParams` without modifying any storage.
+    /// @notice Returns the borrow rate per second (scaled by WAD) of the market `marketParams` without modifying any
+    /// storage.
     /// @dev Assumes that `market` corresponds to `marketParams`.
     function borrowRateView(MarketParams memory marketParams, Market memory market) external view returns (uint256);
 }

src/interfaces/IMorpho.sol

@@ -70,7 +70,7 @@ interface IMorphoBase {
     /// @notice Whether the `lltv` is enabled.
     function isLltvEnabled(uint256 lltv) external view returns (bool);
 
-    /// @notice Whether `authorized` is authorized to modify `authorizer`'s positions.
+    /// @notice Whether `authorized` is authorized to modify `authorizer`'s position on all markets.
     /// @dev Anyone is authorized to modify their own positions, regardless of this variable.
     function isAuthorized(address authorizer, address authorized) external view returns (bool);
 
@@ -91,6 +91,7 @@ interface IMorphoBase {
     function enableLltv(uint256 lltv) external;
 
     /// @notice Sets the `newFee` for the given market `marketParams`.
+    /// @param newFee The new fee, scaled by WAD.
     /// @dev Warning: The recipient can be the zero address.
     function setFee(MarketParams memory marketParams, uint256 newFee) external;
 
@@ -129,9 +130,9 @@ interface IMorphoBase {
 
     /// @notice Supplies `assets` or `shares` on behalf of `onBehalf`, optionally calling back the caller's
     /// `onMorphoSupply` function with the given `data`.
-    /// @dev Either `assets` or `shares` should be zero. Most usecases should rely on `assets` as an input so the caller
-    /// is guaranteed to have `assets` tokens pulled from their balance, but the possibility to mint a specific amount
-    /// of shares is given for full compatibility and precision.
+    /// @dev Either `assets` or `shares` should be zero. Most use cases should rely on `assets` as an input so the
+    /// caller is guaranteed to have `assets` tokens pulled from their balance, but the possibility to mint a specific
+    /// amount of shares is given for full compatibility and precision.
     /// @dev Supplying a large amount can revert for overflow.
     /// @dev Supplying an amount of shares may lead to supply more or fewer assets than expected due to slippage.
     /// Consider using the `assets` parameter to avoid this.
@@ -172,9 +173,9 @@ interface IMorphoBase {
     ) external returns (uint256 assetsWithdrawn, uint256 sharesWithdrawn);
 
     /// @notice Borrows `assets` or `shares` on behalf of `onBehalf` to `receiver`.
-    /// @dev Either `assets` or `shares` should be zero. Most usecases should rely on `assets` as an input so the caller
-    /// is guaranteed to borrow `assets` of tokens, but the possibility to mint a specific amount of shares is given for
-    /// full compatibility and precision.
+    /// @dev Either `assets` or `shares` should be zero. Most use cases should rely on `assets` as an input so the
+    /// caller is guaranteed to borrow `assets` of tokens, but the possibility to mint a specific amount of shares is
+    /// given for full compatibility and precision.
     /// @dev `msg.sender` must be authorized to manage `onBehalf`'s positions.
     /// @dev Borrowing a large amount can revert for overflow.
     /// @dev Borrowing an amount of shares may lead to borrow fewer assets than expected due to slippage.
@@ -200,6 +201,7 @@ interface IMorphoBase {
     /// @dev Repaying an amount corresponding to more shares than borrowed will revert for underflow.
     /// @dev It is advised to use the `shares` input when repaying the full position to avoid reverts due to conversion
     /// roundings between shares and assets.
+    /// @dev An attacker can front-run a repay with a small repay making the transaction revert for underflow.
     /// @param marketParams The market to repay assets to.
     /// @param assets The amount of assets to repay.
     /// @param shares The amount of shares to burn.
@@ -242,6 +244,7 @@ interface IMorphoBase {
     /// @dev Either `seizedAssets` or `repaidShares` should be zero.
     /// @dev Seizing more than the collateral balance will underflow and revert without any error message.
     /// @dev Repaying more than the borrow balance will underflow and revert without any error message.
+    /// @dev An attacker can front-run a liquidation with a small repay making the transaction revert for underflow.
     /// @param marketParams The market of the position.
     /// @param borrower The owner of the position.
     /// @param seizedAssets The amount of collateral to seize.

src/libraries/EventsLib.sol

@@ -35,17 +35,18 @@ library EventsLib {
     event CreateMarket(Id indexed id, MarketParams marketParams);
 
     /// @notice Emitted on supply of assets.
+    /// @dev Warning: `feeRecipient` receives some shares during interest accrual without any supply event emitted.
     /// @param id The market id.
     /// @param caller The caller.
-    /// @param onBehalf The address that received the supply.
+    /// @param onBehalf The owner of the modified position.
     /// @param assets The amount of assets supplied.
     /// @param shares The amount of shares minted.
     event Supply(Id indexed id, address indexed caller, address indexed onBehalf, uint256 assets, uint256 shares);
 
     /// @notice Emitted on withdrawal of assets.
     /// @param id The market id.
     /// @param caller The caller.
-    /// @param onBehalf The address from which the assets were withdrawn.
+    /// @param onBehalf The owner of the modified position.
     /// @param receiver The address that received the withdrawn assets.
     /// @param assets The amount of assets withdrawn.
     /// @param shares The amount of shares burned.
@@ -61,7 +62,7 @@ library EventsLib {
     /// @notice Emitted on borrow of assets.
     /// @param id The market id.
     /// @param caller The caller.
-    /// @param onBehalf The address from which the assets were borrowed.
+    /// @param onBehalf The owner of the modified position.
     /// @param receiver The address that received the borrowed assets.
     /// @param assets The amount of assets borrowed.
     /// @param shares The amount of shares minted.
@@ -77,22 +78,22 @@ library EventsLib {
     /// @notice Emitted on repayment of assets.
     /// @param id The market id.
     /// @param caller The caller.
-    /// @param onBehalf The address for which the assets were repaid.
+    /// @param onBehalf The owner of the modified position.
     /// @param assets The amount of assets repaid. May be 1 over the corresponding market's `totalBorrowAssets`.
     /// @param shares The amount of shares burned.
     event Repay(Id indexed id, address indexed caller, address indexed onBehalf, uint256 assets, uint256 shares);
 
     /// @notice Emitted on supply of collateral.
     /// @param id The market id.
     /// @param caller The caller.
-    /// @param onBehalf The address that received the collateral.
+    /// @param onBehalf The owner of the modified position.
     /// @param assets The amount of collateral supplied.
     event SupplyCollateral(Id indexed id, address indexed caller, address indexed onBehalf, uint256 assets);
 
     /// @notice Emitted on withdrawal of collateral.
     /// @param id The market id.
     /// @param caller The caller.
-    /// @param onBehalf The address from which the collateral was withdrawn.
+    /// @param onBehalf The owner of the modified position.
     /// @param receiver The address that received the withdrawn collateral.
     /// @param assets The amount of collateral withdrawn.
     event WithdrawCollateral(

src/libraries/UtilsLib.sol

@@ -29,7 +29,7 @@ library UtilsLib {
         return uint128(x);
     }
 
-    /// @dev Returns max(x - y, 0).
+    /// @dev Returns max(0, x - y).
     function zeroFloorSub(uint256 x, uint256 y) internal pure returns (uint256 z) {
         assembly {
             z := mul(gt(x, y), sub(x, y))

test/forge/helpers/SigUtils.sol

@@ -12,7 +12,7 @@ library SigUtils {
         pure
         returns (bytes32)
     {
-        return keccak256(abi.encodePacked("\x19\x01", domainSeparator, hashStruct(authorization)));
+        return keccak256(bytes.concat("\x19\x01", domainSeparator, hashStruct(authorization)));
     }
 
     function hashStruct(Authorization memory authorization) internal pure returns (bytes32) {

test/forge/integration/AuthorizationIntegrationTest.sol

@@ -16,11 +16,38 @@ contract AuthorizationIntegrationTest is BaseTest {
         assertFalse(morpho.isAuthorized(address(this), addressFuzz));
     }
 
+    function testAlreadySet(address addressFuzz) public {
+        vm.expectRevert(bytes(ErrorsLib.ALREADY_SET));
+        morpho.setAuthorization(addressFuzz, false);
+
+        morpho.setAuthorization(addressFuzz, true);
+
+        vm.expectRevert(bytes(ErrorsLib.ALREADY_SET));
+        morpho.setAuthorization(addressFuzz, true);
+    }
+
+    function testAlreadySetWithSig(Authorization memory authorization, Signature memory sig) public {
+        authorization.isAuthorized = false;
+        authorization.authorizer = address(this);
+        authorization.deadline = block.timestamp;
+        authorization.nonce = 0;
+
+        vm.expectRevert(bytes(ErrorsLib.ALREADY_SET));
+        morpho.setAuthorizationWithSig(authorization, sig);
+
+        morpho.setAuthorization(authorization.authorized, true);
+
+        authorization.isAuthorized = true;
+        vm.expectRevert(bytes(ErrorsLib.ALREADY_SET));
+        morpho.setAuthorizationWithSig(authorization, sig);
+    }
+
     function testSetAuthorizationWithSignatureDeadlineOutdated(
         Authorization memory authorization,
         uint256 privateKey,
         uint256 blocks
     ) public {
+        authorization.isAuthorized = true;
         blocks = _boundBlocks(blocks);
         authorization.deadline = block.timestamp - 1;
 
@@ -40,6 +67,7 @@ contract AuthorizationIntegrationTest is BaseTest {
     }
 
     function testAuthorizationWithSigWrongPK(Authorization memory authorization, uint256 privateKey) public {
+        authorization.isAuthorized = true;
         authorization.deadline = bound(authorization.deadline, block.timestamp, type(uint256).max);
 
         // Private key must be less than the secp256k1 curve order.
@@ -55,6 +83,7 @@ contract AuthorizationIntegrationTest is BaseTest {
     }
 
     function testAuthorizationWithSigWrongNonce(Authorization memory authorization, uint256 privateKey) public {
+        authorization.isAuthorized = true;
         authorization.deadline = bound(authorization.deadline, block.timestamp, type(uint256).max);
         authorization.nonce = bound(authorization.nonce, 1, type(uint256).max);
 
@@ -71,6 +100,7 @@ contract AuthorizationIntegrationTest is BaseTest {
     }
 
     function testAuthorizationWithSig(Authorization memory authorization, uint256 privateKey) public {
+        authorization.isAuthorized = true;
         authorization.deadline = bound(authorization.deadline, block.timestamp, type(uint256).max);
 
         // Private key must be less than the secp256k1 curve order.
@@ -84,11 +114,12 @@ contract AuthorizationIntegrationTest is BaseTest {
 
         morpho.setAuthorizationWithSig(authorization, sig);
 
-        assertEq(morpho.isAuthorized(authorization.authorizer, authorization.authorized), authorization.isAuthorized);
+        assertEq(morpho.isAuthorized(authorization.authorizer, authorization.authorized), true);
         assertEq(morpho.nonce(authorization.authorizer), 1);
     }
 
     function testAuthorizationFailsWithReusedSig(Authorization memory authorization, uint256 privateKey) public {
+        authorization.isAuthorized = true;
         authorization.deadline = bound(authorization.deadline, block.timestamp, type(uint256).max);
 
         // Private key must be less than the secp256k1 curve order.
@@ -102,6 +133,7 @@ contract AuthorizationIntegrationTest is BaseTest {
 
         morpho.setAuthorizationWithSig(authorization, sig);
 
+        authorization.isAuthorized = false;
         vm.expectRevert(bytes(ErrorsLib.INVALID_NONCE));
         morpho.setAuthorizationWithSig(authorization, sig);
     }

test/forge/integration/BorrowIntegrationTest.sol

@@ -202,7 +202,7 @@ contract BorrowIntegrationTest is BaseTest {
         vm.startPrank(ONBEHALF);
         collateralToken.approve(address(morpho), amountCollateral);
         morpho.supplyCollateral(marketParams, amountCollateral, ONBEHALF, hex"");
-        morpho.setAuthorization(BORROWER, true);
+        // BORROWER is already authorized.
         vm.stopPrank();
 
         uint256 expectedBorrowShares = amountBorrowed.toSharesUp(0, 0);
@@ -248,7 +248,7 @@ contract BorrowIntegrationTest is BaseTest {
         vm.startPrank(ONBEHALF);
         collateralToken.approve(address(morpho), amountCollateral);
         morpho.supplyCollateral(marketParams, amountCollateral, ONBEHALF, hex"");
-        morpho.setAuthorization(BORROWER, true);
+        // BORROWER is already authorized.
         vm.stopPrank();
 
         vm.prank(BORROWER);

test/forge/integration/CallbacksIntegrationTest.sol

@@ -83,6 +83,11 @@ contract CallbacksIntegrationTest is
         assertEq(loanToken.balanceOf(address(morpho)), amount, "balanceOf");
     }
 
+    function testFlashLoanZero() public {
+        vm.expectRevert(bytes(ErrorsLib.ZERO_ASSETS));
+        morpho.flashLoan(address(loanToken), 0, abi.encode(this.testFlashLoan.selector, hex""));
+    }
+
     function testFlashLoanShouldRevertIfNotReimbursed(uint256 amount) public {
         amount = bound(amount, 1, MAX_TEST_AMOUNT);
 

test/forge/integration/WithdrawCollateralIntegrationTest.sol

@@ -145,7 +145,7 @@ contract WithdrawCollateralIntegrationTest is BaseTest {
 
         vm.startPrank(ONBEHALF);
         morpho.supplyCollateral(marketParams, amountCollateral + amountCollateralExcess, ONBEHALF, hex"");
-        morpho.setAuthorization(BORROWER, true);
+        // BORROWER is already authorized.
         morpho.borrow(marketParams, amountBorrowed, 0, ONBEHALF, ONBEHALF);
         vm.stopPrank();
 

test/forge/invariant/MorphoInvariantTest.sol

@@ -62,15 +62,17 @@ contract MorphoInvariantTest is InvariantTest {
     }
 
     modifier authorized(address onBehalf) {
-        if (onBehalf != msg.sender) {
+        if (onBehalf != msg.sender && !morpho.isAuthorized(onBehalf, msg.sender)) {
             vm.prank(onBehalf);
             morpho.setAuthorization(msg.sender, true);
         }
 
         _;
 
-        vm.prank(onBehalf);
-        morpho.setAuthorization(msg.sender, false);
+        if (morpho.isAuthorized(onBehalf, msg.sender)) {
+            vm.prank(onBehalf);
+            morpho.setAuthorization(msg.sender, false);
+        }
     }
 
     function _randomMarket(uint256 marketSeed) internal view returns (MarketParams memory _marketParams) {