diff --git a/src/Morpho.sol b/src/Morpho.sol index 7871519af..90205cd09 100644 --- a/src/Morpho.sol +++ b/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,6 +434,8 @@ 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); @@ -439,11 +443,15 @@ contract Morpho is IMorphoStaticTyping { /// @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); diff --git a/src/interfaces/IIrm.sol b/src/interfaces/IIrm.sol index db2aaf873..3de0bc1e8 100644 --- a/src/interfaces/IIrm.sol +++ b/src/interfaces/IIrm.sol @@ -8,11 +8,12 @@ import {MarketParams, Market} from "./IMorpho.sol"; /// @custom:contact security@morpho.org /// @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); } diff --git a/src/interfaces/IMorpho.sol b/src/interfaces/IMorpho.sol index 502bdd31b..83b12302b 100644 --- a/src/interfaces/IMorpho.sol +++ b/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. diff --git a/src/libraries/EventsLib.sol b/src/libraries/EventsLib.sol index 166f4d296..2ff9b829d 100644 --- a/src/libraries/EventsLib.sol +++ b/src/libraries/EventsLib.sol @@ -35,9 +35,10 @@ 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); @@ -45,7 +46,7 @@ library EventsLib { /// @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,7 +78,7 @@ 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); @@ -85,14 +86,14 @@ library EventsLib { /// @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( diff --git a/src/libraries/UtilsLib.sol b/src/libraries/UtilsLib.sol index 066043d13..f343ef769 100644 --- a/src/libraries/UtilsLib.sol +++ b/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)) diff --git a/test/forge/helpers/SigUtils.sol b/test/forge/helpers/SigUtils.sol index 9a60b52f0..b8e90ea51 100644 --- a/test/forge/helpers/SigUtils.sol +++ b/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) { diff --git a/test/forge/integration/AuthorizationIntegrationTest.sol b/test/forge/integration/AuthorizationIntegrationTest.sol index 263eaa550..e7933d19f 100644 --- a/test/forge/integration/AuthorizationIntegrationTest.sol +++ b/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); } diff --git a/test/forge/integration/BorrowIntegrationTest.sol b/test/forge/integration/BorrowIntegrationTest.sol index 1733c7872..d02a83efb 100644 --- a/test/forge/integration/BorrowIntegrationTest.sol +++ b/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); diff --git a/test/forge/integration/CallbacksIntegrationTest.sol b/test/forge/integration/CallbacksIntegrationTest.sol index 62578aa19..512522e5e 100644 --- a/test/forge/integration/CallbacksIntegrationTest.sol +++ b/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); diff --git a/test/forge/integration/WithdrawCollateralIntegrationTest.sol b/test/forge/integration/WithdrawCollateralIntegrationTest.sol index 8a5b0ce58..c9798d0ad 100644 --- a/test/forge/integration/WithdrawCollateralIntegrationTest.sol +++ b/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(); diff --git a/test/forge/invariant/MorphoInvariantTest.sol b/test/forge/invariant/MorphoInvariantTest.sol index edb423473..122d5f515 100644 --- a/test/forge/invariant/MorphoInvariantTest.sol +++ b/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) {