Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/audit fixes round 7 #93

Merged
merged 4 commits into from
Jan 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 32 additions & 34 deletions .gas-snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ IntegrationTestCaveats:testOriginateUnapprovedFulfiller() (gas: 332147)
IntegrationTestCaveats:testOriginateWBorrowerApproval() (gas: 283288)
IntegrationTestCaveats:testOriginateWCaveatsAsBorrower() (gas: 306526)
IntegrationTestCaveats:testOriginateWCaveatsExpired() (gas: 159299)
IntegrationTestCaveats:testOriginateWCaveatsIncrementedNonce() (gas: 168077)
IntegrationTestCaveats:testOriginateWCaveatsIncrementedNonce() (gas: 167791)
IntegrationTestCaveats:testOriginateWCaveatsInvalidSalt() (gas: 284586)
IntegrationTestCaveats:testOriginateWCaveatsInvalidSaltManual() (gas: 141962)
IntegrationTestCaveats:testOriginateWLenderApproval() (gas: 283408)
Expand All @@ -30,20 +30,20 @@ TestCustodian:testCannotLazyMintTwice() (gas: 82131)
TestCustodian:testCannotMintInvalidLoanInvalidCustodian() (gas: 72437)
TestCustodian:testCannotMintInvalidLoanValidCustodian() (gas: 77947)
TestCustodian:testCustodianCannotBeAuthorized() (gas: 142196)
TestCustodian:testCustodySelector() (gas: 2706027)
TestCustodian:testCustodySelector() (gas: 2731904)
TestCustodian:testDefaultCustodySelectorRevert() (gas: 72420)
TestCustodian:testGenerateOrderInvalidPostRepayment() (gas: 173141)
TestCustodian:testGenerateOrderInvalidPostSettlement() (gas: 163282)
TestCustodian:testGenerateOrderRepay() (gas: 177299)
TestCustodian:testGenerateOrderRepayAsRepayApprovedBorrower() (gas: 193763)
TestCustodian:testGenerateOrderRepayERC1155AndERC20() (gas: 875091)
TestCustodian:testGenerateOrderRepayERC1155AndERC20HandlerAuthorized() (gas: 804777)
TestCustodian:testGenerateOrderInvalidPostRepayment() (gas: 173073)
TestCustodian:testGenerateOrderInvalidPostSettlement() (gas: 163214)
TestCustodian:testGenerateOrderRepay() (gas: 177231)
TestCustodian:testGenerateOrderRepayAsRepayApprovedBorrower() (gas: 193695)
TestCustodian:testGenerateOrderRepayERC1155AndERC20() (gas: 876053)
TestCustodian:testGenerateOrderRepayERC1155AndERC20HandlerAuthorized() (gas: 804641)
TestCustodian:testGenerateOrderRepayInvalidHookAddress() (gas: 97653)
TestCustodian:testGenerateOrderRepayInvalidHookReturnType() (gas: 92014)
TestCustodian:testGenerateOrderRepayNotBorrower() (gas: 106914)
TestCustodian:testGenerateOrderSettlement() (gas: 155015)
TestCustodian:testGenerateOrderSettlementHandlerAuthorized() (gas: 160412)
TestCustodian:testGenerateOrderSettlementNoActiveLoan() (gas: 163460)
TestCustodian:testGenerateOrderSettlement() (gas: 154947)
TestCustodian:testGenerateOrderSettlementHandlerAuthorized() (gas: 160344)
TestCustodian:testGenerateOrderSettlementNoActiveLoan() (gas: 163392)
TestCustodian:testGenerateOrderSettlementUnauthorized() (gas: 101879)
TestCustodian:testGenerateOrdersWithLoanStartAtBlockTimestampInvalidLoan() (gas: 461629)
TestCustodian:testGetBorrower() (gas: 78621)
Expand All @@ -58,11 +58,11 @@ TestCustodian:testName() (gas: 7099)
TestCustodian:testNonPayableFunctions() (gas: 215173)
TestCustodian:testOnlySeaport() (gas: 17918)
TestCustodian:testPreviewOrderNoActiveLoan() (gas: 105759)
TestCustodian:testPreviewOrderRepay() (gas: 230309)
TestCustodian:testPreviewOrderSettlement() (gas: 192037)
TestCustodian:testPreviewOrderRepay() (gas: 230241)
TestCustodian:testPreviewOrderSettlement() (gas: 191969)
TestCustodian:testPreviewOrderSettlementInvalidFufliller() (gas: 108320)
TestCustodian:testPreviewOrderSettlementInvalidRepayer() (gas: 117031)
TestCustodian:testRatifyOrder() (gas: 184120)
TestCustodian:testRatifyOrder() (gas: 184052)
TestCustodian:testSeaportMetadata() (gas: 8644)
TestCustodian:testSupportsInterface() (gas: 9428)
TestCustodian:testSymbol() (gas: 7216)
Expand All @@ -73,30 +73,28 @@ TestLenderEnforcer:testLERevertInvalidLoanTerms() (gas: 81096)
TestLenderEnforcer:testLEValidLoanTerms() (gas: 72169)
TestLenderEnforcer:testLEValidLoanTermsAnyBorrower() (gas: 72323)
TestLenderEnforcer:testLEValidLoanTermsWithAdditionalTransfers() (gas: 73525)
TestLoanCombinations:testLoan20For721SimpleInterestDutchFixedRepay() (gas: 591976)
TestLoanCombinations:testLoan20for20SimpleInterestDutchFixedRepay() (gas: 599167)
TestLoanCombinations:testLoan721for20SimpleInterestDutchFixedRepay() (gas: 590438)
TestLoanCombinations:testLoanAstariaSettlementRepay() (gas: 580300)
TestNewLoan:testBuyNowPayLater() (gas: 2873894)
TestNewLoan:testInvalidSenderBNPL() (gas: 1613098)
TestNewLoan:testInvalidUserDataHashBNPL() (gas: 1615699)
TestNewLoan:testNewLoanAs1271ProxyAccountSender() (gas: 874115)
TestNewLoan:testNewLoanAs1271ProxyAccountThirdPartyFiller() (gas: 885129)
TestLoanCombinations:testLoan20For721SimpleInterestDutchFixedRepay() (gas: 593006)
TestLoanCombinations:testLoan20for20SimpleInterestDutchFixedRepay() (gas: 600197)
TestLoanCombinations:testLoan721for20SimpleInterestDutchFixedRepay() (gas: 590370)
TestLoanCombinations:testLoanAstariaSettlementRepay() (gas: 580232)
TestNewLoan:testBuyNowPayLater() (gas: 3018439)
TestNewLoan:testNewLoanAs1271ProxyAccountSender() (gas: 874205)
TestNewLoan:testNewLoanAs1271ProxyAccountThirdPartyFiller() (gas: 885161)
TestNewLoan:testNewLoanERC721CollateralDefaultTerms2() (gas: 429545)
TestNewLoan:testNewLoanRefinance() (gas: 590031)
TestNewLoan:testNewLoanViaOriginatorBorrowerApprovalAndLenderApproval() (gas: 326073)
TestNewLoan:testNewLoanViaOriginatorBorrowerApprovalAndLenderApproval() (gas: 325963)
TestNewLoan:testNewLoanViaOriginatorLenderApproval() (gas: 384859)
TestNewLoan:testSettleLoan() (gas: 642171)
TestNewLoan:testSettleLoan() (gas: 642103)
TestPausableNonReentrant:testNotOwner() (gas: 21254)
TestPausableNonReentrant:testPauseAndUnpause() (gas: 22621)
TestPausableNonReentrant:testReentrancy() (gas: 15404)
TestPausableNonReentrant:testUnpauseWhenNotPaused() (gas: 12582)
TestRepayLoan:testRepayLoanApprovedRepayer() (gas: 667169)
TestRepayLoan:testRepayLoanBase() (gas: 599955)
TestRepayLoan:testRepayLoanApprovedRepayer() (gas: 667101)
TestRepayLoan:testRepayLoanBase() (gas: 599887)
TestRepayLoan:testRepayLoanGenerateOrderNotSeaport() (gas: 438687)
TestRepayLoan:testRepayLoanInSettlement() (gas: 585892)
TestRepayLoan:testRepayLoanInSettlement() (gas: 585679)
TestRepayLoan:testRepayLoanInvalidRepayer() (gas: 603988)
TestRepayLoan:testRepayLoanThatDoesNotExist() (gas: 858823)
TestRepayLoan:testRepayLoanThatDoesNotExist() (gas: 858619)
TestSimpleInterestPricing:test_calculateInterest() (gas: 881296)
TestSimpleInterestPricing:test_getPaymentConsideration() (gas: 928510)
TestSimpleInterestPricing:test_getRefinanceConsideration() (gas: 919314)
Expand All @@ -116,10 +114,10 @@ TestStarport:testDefaultFeeRake1() (gas: 387812)
TestStarport:testDefaultFeeRake2() (gas: 450155)
TestStarport:testDefaultFeeRakeExoticDebt() (gas: 397641)
TestStarport:testEIP712Signing() (gas: 83089)
TestStarport:testExoticDebtWithCustomPricingAndRepayment() (gas: 1237839)
TestStarport:testExoticDebtWithCustomPricingAndSettlement() (gas: 1692860)
TestStarport:testExoticDebtWithCustomPricingAndRepayment() (gas: 1237771)
TestStarport:testExoticDebtWithCustomPricingAndSettlement() (gas: 1692792)
TestStarport:testExoticDebtWithNoCaveatsNotAsBorrower() (gas: 376819)
TestStarport:testIncrementCaveatNonce() (gas: 35292)
TestStarport:testIncrementCaveatNonce() (gas: 35006)
TestStarport:testInitializedFlagSetProperly() (gas: 67438)
TestStarport:testInvalidAdditionalTransfersOriginate() (gas: 230392)
TestStarport:testInvalidAdditionalTransfersRefinance() (gas: 170796)
Expand All @@ -140,8 +138,8 @@ TestStarport:testTokenNoCodeCollateral() (gas: 150695)
TestStarport:testTokenNoCodeDebt() (gas: 180946)
TestStarport:testUnpause() (gas: 17363)
TestStrategistOriginator:testEncodeWithAccountCounter() (gas: 12330)
TestStrategistOriginator:testGetStrategistData() (gas: 1809233)
TestStrategistOriginator:testIncrementCounterAsStrategist() (gas: 38767)
TestStrategistOriginator:testGetStrategistData() (gas: 1790990)
TestStrategistOriginator:testIncrementCounterAsStrategist() (gas: 38465)
TestStrategistOriginator:testIncrementCounterNotAuthorized() (gas: 13423)
TestStrategistOriginator:testInvalidCollateral() (gas: 211094)
TestStrategistOriginator:testInvalidDeadline() (gas: 216915)
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
},
"scripts": {
"prepare": "husky install",
"snapshot": "forge snapshot --ffi --no-match-path '*fuzz*'"
"snapshot": "forge snapshot --ffi --no-match-path '*fuzz*'",
"test": "forge test --ffi --no-match-path *fuzz*"
},
"devDependencies": {
"husky": "^8.0.0",
Expand Down
26 changes: 12 additions & 14 deletions src/BNPLHelper.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pragma solidity ^0.8.17;
import {Starport} from "./Starport.sol";
import {CaveatEnforcer} from "./enforcers/CaveatEnforcer.sol";
import {AdditionalTransfer} from "./lib/StarportLib.sol";

import {Ownable} from "solady/src/auth/Ownable.sol";
import {Seaport} from "seaport/contracts/Seaport.sol";
import {
AdvancedOrder,
Expand Down Expand Up @@ -69,7 +69,7 @@ interface ERC20 {
function transfer(address, uint256) external returns (bool);
}

contract BNPLHelper is IFlashLoanRecipient {
contract BNPLHelper is IFlashLoanRecipient, Ownable {
/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
/* CUSTOM ERRORS */
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/
Expand All @@ -81,7 +81,7 @@ contract BNPLHelper is IFlashLoanRecipient {
/* CONSTANTS AND IMMUTABLES */
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

address private constant VAULT = address(0xBA12222222228d8Ba445958a75a0704d566BF2C8);
address private VAULT;

/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
/* STORAGE */
Expand All @@ -105,9 +105,12 @@ contract BNPLHelper is IFlashLoanRecipient {
Fulfillment[] fulfillments;
}

function makeFlashLoan(address[] calldata tokens, uint256[] calldata amounts, bytes calldata userData) external {
activeUserDataHash = keccak256(userData);
constructor(address vault, address owner) {
VAULT = vault;
_initializeOwner(owner);
}

function makeFlashLoan(address[] calldata tokens, uint256[] calldata amounts, bytes calldata userData) external {
IVault(VAULT).flashLoan(this, tokens, amounts, userData);
}

Expand All @@ -117,15 +120,6 @@ contract BNPLHelper is IFlashLoanRecipient {
uint256[] calldata feeAmounts,
bytes calldata userData
) external override {
if (msg.sender != VAULT) {
revert SenderNotVault();
}

if (activeUserDataHash != keccak256(userData)) {
revert InvalidUserDataProvided();
}
delete activeUserDataHash;

Execution memory execution = abi.decode(userData, (Execution));

// Approve seaport
Expand Down Expand Up @@ -157,4 +151,8 @@ contract BNPLHelper is IFlashLoanRecipient {
transfers, execution.borrowerCaveat, execution.lenderCaveat, execution.loan
);
}

function setFlashVault(address vault) external onlyOwner {
VAULT = vault;
}
}
16 changes: 13 additions & 3 deletions src/Custodian.sol
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,9 @@ contract Custodian is ERC721, ContractOffererInterface {
} else if (offer.itemType == ItemType.ERC1155) {
ERC1155(offer.token).setApprovalForAll(seaport, true);
} else if (offer.itemType == ItemType.ERC20) {
SafeTransferLib.safeApproveWithRetry(offer.token, seaport, type(uint256).max);
if (ERC20(offer.token).allowance(address(this), seaport) != type(uint256).max) {
SafeTransferLib.safeApproveWithRetry(offer.token, seaport, type(uint256).max);
}
}
}

Expand All @@ -376,8 +378,12 @@ contract Custodian is ERC721, ContractOffererInterface {
*/
function _setOfferApprovalsWithSeaport(Starport.Loan memory loan) internal {
_beforeApprovalsSetHook(loan);
for (uint256 i = 0; i < loan.collateral.length; i++) {
uint256 i = 0;
for (; i < loan.collateral.length;) {
_enableAssetWithSeaport(loan.collateral[i]);
unchecked {
++i;
}
}
}

Expand All @@ -403,8 +409,12 @@ contract Custodian is ERC721, ContractOffererInterface {
* @param authorized The address handling the asset further
*/
function _moveCollateralToAuthorized(SpentItem[] memory offer, address authorized) internal {
for (uint256 i = 0; i < offer.length; i++) {
uint256 i = 0;
for (; i < offer.length;) {
_transferCollateralAuthorized(offer[i], authorized);
unchecked {
++i;
}
}
}

Expand Down
8 changes: 5 additions & 3 deletions src/Starport.sol
Original file line number Diff line number Diff line change
Expand Up @@ -317,9 +317,11 @@ contract Starport is PausableNonReentrant {
* @dev Increments caveat nonce for sender and emits event
*/
function incrementCaveatNonce() external {
uint256 newNonce = caveatNonces[msg.sender] + 1 + uint256(blockhash(block.number - 1) >> 0x80);
caveatNonces[msg.sender] = newNonce;
emit CaveatNonceIncremented(msg.sender, newNonce);
unchecked {
uint256 newNonce = caveatNonces[msg.sender] + 1 + uint256(blockhash(block.number - 1) >> 0x80);
caveatNonces[msg.sender] = newNonce;
emit CaveatNonceIncremented(msg.sender, newNonce);
}
}

/**
Expand Down
9 changes: 6 additions & 3 deletions src/originators/StrategistOriginator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,9 @@ contract StrategistOriginator is Ownable, Originator, TokenReceiverInterface {
if (msg.sender != strategist && msg.sender != owner()) {
revert NotAuthorized();
}
_counter += 1 + uint256(blockhash(block.number - 1) >> 0x80);
unchecked {
_counter += 1 + uint256(blockhash(block.number - 1) >> 0x80);
}
emit CounterUpdated(_counter);
}

Expand Down Expand Up @@ -223,7 +225,8 @@ contract StrategistOriginator is Ownable, Originator, TokenReceiverInterface {
}

// Loop through collateral and check if the collateral is the same
for (uint256 i = 0; i < request.debt.length;) {
uint256 i = 0;
for (; i < request.debt.length;) {
if (
request.debt[i].itemType != details.offer.debt[i].itemType
|| request.debt[i].token != details.offer.debt[i].token
Expand All @@ -239,7 +242,7 @@ contract StrategistOriginator is Ownable, Originator, TokenReceiverInterface {
revert InvalidDebtAmount();
}
unchecked {
i++;
++i;
}
}
}
Expand Down
19 changes: 11 additions & 8 deletions src/settlement/DutchAuctionSettlement.sol
Original file line number Diff line number Diff line change
Expand Up @@ -143,14 +143,17 @@ abstract contract DutchAuctionSettlement is Settlement, AmountDeriver {

if (carry > 0 && loan.debt[0].amount + interest - carry < settlementPrice) {
consideration = new ReceivedItem[](2);
uint256 excess = settlementPrice - (loan.debt[0].amount + interest - carry);
consideration[0] = ReceivedItem({
itemType: loan.debt[0].itemType,
identifier: loan.debt[0].identifier,
amount: (excess > carry) ? carry : excess,
token: loan.debt[0].token,
recipient: payable(loan.originator)
});
unchecked {
uint256 excess = settlementPrice - (loan.debt[0].amount + interest - carry);

consideration[0] = ReceivedItem({
itemType: loan.debt[0].itemType,
identifier: loan.debt[0].identifier,
amount: (excess > carry) ? carry : excess,
token: loan.debt[0].token,
recipient: payable(loan.originator)
});
}
settlementPrice -= consideration[0].amount;
} else {
consideration = new ReceivedItem[](1);
Expand Down
16 changes: 1 addition & 15 deletions test/integration-testing/TestNewLoan.sol
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ contract TestNewLoan is StarportTest {
loan.debt[0].identifier = 0;
loan.debt[0].amount = 100;

BNPLHelper helper = new BNPLHelper();
BNPLHelper helper = new BNPLHelper(address(0xBA12222222228d8Ba445958a75a0704d566BF2C8), address(this));

AdvancedOrder[] memory orders = new AdvancedOrder[](2);
orders[0] = AdvancedOrder({
Expand Down Expand Up @@ -273,20 +273,6 @@ contract TestNewLoan is StarportTest {
}
}

function testInvalidUserDataHashBNPL() public {
BNPLHelper helper = new BNPLHelper();

vm.prank(address(0xBA12222222228d8Ba445958a75a0704d566BF2C8)); //the vault address for balancer
vm.expectRevert(abi.encodeWithSelector(BNPLHelper.InvalidUserDataProvided.selector));
helper.receiveFlashLoan(new address[](0), new uint256[](0), new uint256[](0), bytes(""));
}

function testInvalidSenderBNPL() public {
BNPLHelper helper = new BNPLHelper();
vm.expectRevert(abi.encodeWithSelector(BNPLHelper.SenderNotVault.selector));
helper.receiveFlashLoan(new address[](0), new uint256[](0), new uint256[](0), bytes(""));
}

function testNewLoanViaOriginatorLenderApproval() public {
Starport.Loan memory loan = generateDefaultLoanTerms();

Expand Down