From e494dae058b9c6b85a86c31752187d7511bb4c6e Mon Sep 17 00:00:00 2001 From: GregTheDev <40359730+0xgregthedev@users.noreply.github.com> Date: Tue, 9 Jan 2024 12:07:47 -0700 Subject: [PATCH] update v1Status and BaseRecall to validate pricing address (#27) * update v1Status and BaseRecall to validate pricing address * add tests, update dependencies * improve tests --- lib/forge-std | 2 +- src/status/AstariaV1Status.sol | 28 ++++++++++++++++--- src/status/BaseRecall.sol | 13 +++++++++ test/AstariaV1Test.sol | 2 ++ test/TestV1Status.sol | 49 ++++++++++++++++++++++++++++++++++ 5 files changed, 90 insertions(+), 4 deletions(-) diff --git a/lib/forge-std b/lib/forge-std index f73c73d..87a2a0a 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit f73c73d2018eb6a111f35e4dae7b4f27401e9421 +Subproject commit 87a2a0afc5fafd6297538a45a52ac19e71a84562 diff --git a/src/status/AstariaV1Status.sol b/src/status/AstariaV1Status.sol index 7d53f12..6bd7b09 100644 --- a/src/status/AstariaV1Status.sol +++ b/src/status/AstariaV1Status.sol @@ -19,15 +19,22 @@ import {Validation} from "starport-core/lib/Validation.sol"; import {BasePricing} from "starport-core/pricing/BasePricing.sol"; import {BaseRecall} from "v1-core/status/BaseRecall.sol"; import {BaseStatus} from "v1-core/status/BaseStatus.sol"; +import {Ownable} from "solady/src/auth/Ownable.sol"; -contract AstariaV1Status is BaseStatus, BaseRecall { +contract AstariaV1Status is BaseStatus, BaseRecall, Ownable { using {StarportLib.getId} for Starport.Loan; + mapping(address => bool) public isValidPricing; + + error InvalidPricingContract(); + /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/ /* CONSTRUCTOR */ /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/ - constructor(Starport SP_) BaseRecall(SP_) {} + constructor(Starport SP_) BaseRecall(SP_) { + _initializeOwner(msg.sender); + } /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/ /* EXTERNAL FUNCTIONS */ @@ -54,9 +61,24 @@ contract AstariaV1Status is BaseStatus, BaseRecall { Details memory details = abi.decode(loan.terms.statusData, (Details)); BasePricing.Details memory pDetails = abi.decode(loan.terms.pricingData, (BasePricing.Details)); bool valid = true; - if (details.recallerRewardRatio > 10 ** pDetails.decimals || details.recallMax > 10 * 10 ** pDetails.decimals) { + if ( + details.recallerRewardRatio > 10 ** pDetails.decimals || details.recallMax > 10 * 10 ** pDetails.decimals + || !isValidPricing[loan.terms.pricing] + ) { valid = false; } + return valid ? Validation.validate.selector : bytes4(0xFFFFFFFF); } + + // @inheritdoc BaseRecall + function validatePricingContract(address pricingContract) internal virtual override { + if (!isValidPricing[pricingContract]) { + revert InvalidPricingContract(); + } + } + + function setValidPricing(address pricing, bool valid) external onlyOwner { + isValidPricing[pricing] = valid; + } } diff --git a/src/status/BaseRecall.sol b/src/status/BaseRecall.sol index 32386c4..11e50d6 100644 --- a/src/status/BaseRecall.sol +++ b/src/status/BaseRecall.sol @@ -21,6 +21,7 @@ import {ItemType} from "seaport-types/src/lib/ConsiderationEnums.sol"; import {ConsiderationInterface} from "seaport-types/src/interfaces/ConsiderationInterface.sol"; import {ERC20} from "solady/src/tokens/ERC20.sol"; import {FixedPointMathLib} from "solady/src/utils/FixedPointMathLib.sol"; +import {Ownable} from "solady/src/auth/Ownable.sol"; abstract contract BaseRecall { using FixedPointMathLib for uint256; @@ -109,6 +110,14 @@ abstract contract BaseRecall { return (details.recallMax * ratio) / baseAdjustment; } + /** + * @dev Implement to validate the pricing contract. + * @dev Malicious pricing contracts can lie about the withdrawable recall amount. + * @dev This function should revert if the pricing contract is invalid. + * @param pricingContract The pricing contract to validate + */ + function validatePricingContract(address pricingContract) internal virtual; + /** * @dev Recalls a loan * @param loan The loan to recall @@ -129,6 +138,8 @@ abstract contract BaseRecall { revert RecallAlreadyExists(); } + validatePricingContract(loan.terms.pricing); + AdditionalTransfer[] memory recallConsideration = _generateRecallConsideration( msg.sender, loan, 0, details.recallStakeDuration, 0, msg.sender, payable(address(this)) ); @@ -154,6 +165,8 @@ abstract contract BaseRecall { revert LoanHasNotBeenRefinanced(); } + validatePricingContract(loan.terms.pricing); + Recall storage recall = recalls[loanId]; address recaller = recall.recaller; // Ensure that a recall exists for the provided tokenId, ensure that the recall diff --git a/test/AstariaV1Test.sol b/test/AstariaV1Test.sol index d978f1b..c8049ea 100644 --- a/test/AstariaV1Test.sol +++ b/test/AstariaV1Test.sol @@ -40,8 +40,10 @@ contract AstariaV1Test is StarportTest { vm.label(address(pricing), "V1Pricing"); settlement = new AstariaV1Settlement(SP); vm.label(address(settlement), "V1Settlement"); + status = new AstariaV1Status(SP); vm.label(address(status), "V1Status"); + AstariaV1Status(address(status)).setValidPricing(address(pricing), true); lenderEnforcer = new AstariaV1LenderEnforcer(); vm.label(address(lenderEnforcer), "V1LenderEnforcer"); diff --git a/test/TestV1Status.sol b/test/TestV1Status.sol index 4088a47..47a68ac 100644 --- a/test/TestV1Status.sol +++ b/test/TestV1Status.sol @@ -22,6 +22,7 @@ import {DeepEq} from "starport-test/utils/DeepEq.sol"; import {SpentItemLib} from "seaport-sol/src/lib/SpentItemLib.sol"; import {FixedPointMathLib} from "solady/src/utils/FixedPointMathLib.sol"; import {Validation} from "starport-core/lib/Validation.sol"; +import {Ownable} from "solady/src/auth/Ownable.sol"; contract TestAstariaV1Status is AstariaV1Test, DeepEq { using Cast for *; @@ -75,6 +76,54 @@ contract TestAstariaV1Status is AstariaV1Test, DeepEq { assert(AstariaV1Status(loan.terms.status).isRecalled(loan)); } + function testV1StatusRevertInvalidPricing() public { + AstariaV1Status v1Status = AstariaV1Status(address(status)); + vm.prank(v1Status.owner()); + v1Status.setValidPricing(address(pricing), false); + + Starport.Terms memory terms = Starport.Terms({ + status: address(status), + settlement: address(settlement), + pricing: address(pricing), + pricingData: defaultPricingData, + settlementData: defaultSettlementData, + statusData: defaultStatusData + }); + Starport.Loan memory loan = + _createLoan721Collateral20Debt({lender: lender.addr, borrowAmount: 1e18, terms: terms}); + uint256 loanId = loan.getId(); + + BaseRecall.Details memory details = abi.decode(loan.terms.statusData, (BaseRecall.Details)); + + erc20s[0].mint(address(this), 10e18); + erc20s[0].approve(loan.terms.status, 10e18); + + skip(details.honeymoon); + vm.expectRevert(AstariaV1Status.InvalidPricingContract.selector); + v1Status.recall(loan); + } + + function testSetValidPricing() public { + AstariaV1Status v1Status = AstariaV1Status(address(status)); + vm.startPrank(v1Status.owner()); + v1Status.setValidPricing(address(0xdead), true); + assertTrue(v1Status.isValidPricing(address(0xdead))); + v1Status.setValidPricing(address(0xdead), false); + assertFalse(v1Status.isValidPricing(address(0xdead))); + vm.stopPrank(); + + vm.prank(v1Status.owner()); + v1Status.transferOwnership(address(0xdead)); + assertEq(v1Status.owner(), address(0xdead)); + + vm.prank(address(0xdead)); + v1Status.setValidPricing(address(0xdead), true); + assertTrue(v1Status.isValidPricing(address(0xdead))); + + vm.expectRevert(Ownable.Unauthorized.selector); + v1Status.setValidPricing(address(0xdead), true); + } + function testRecallAndRefinanceInsideWindow() public { Starport.Terms memory terms = Starport.Terms({ status: address(status),