Skip to content

Commit

Permalink
Avoid permit when maker is a smart contract (EIP-2612 does not suppor…
Browse files Browse the repository at this point in the history
…t that)
  • Loading branch information
k06a committed Oct 2, 2023
1 parent d39a5be commit db7fafd
Showing 1 changed file with 9 additions and 22 deletions.
31 changes: 9 additions & 22 deletions contracts/OrderMixin.sol
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,15 @@ abstract contract OrderMixin is IOrderMixin, EIP712, OnlyWethReceiver, Predicate
address maker = order.maker.get();
if (maker == address(0) || maker != ECDSA.recover(orderHash, r, vs)) revert BadSignature();
if (!takerTraits.skipMakerPermit()) {
_applyMakerPermit(order, orderHash, maker, extension);
bytes calldata makerPermit = extension.makerPermit();
if (makerPermit.length >= 20) {
// proceed only if taker is willing to execute permit and its length is enough to store address
IERC20(address(bytes20(makerPermit))).tryPermit(maker, address(this), makerPermit[20:]);
if (!order.makerTraits.useBitInvalidator()) {
// Bit orders are not subjects for reentrancy, but we still need to check remaining-based orders for reentrancy
if (!_remainingInvalidator[order.maker.get()][orderHash].isNewOrder()) revert ReentrancyDetected();
}
}
}
}

Expand Down Expand Up @@ -223,9 +231,6 @@ abstract contract OrderMixin is IOrderMixin, EIP712, OnlyWethReceiver, Predicate
uint256 remainingMakingAmount = _checkRemainingMakingAmount(order, orderHash);
if (remainingMakingAmount == order.makingAmount) {
if (!ECDSA.isValidSignature(order.maker.get(), orderHash, signature)) revert BadSignature();
if (!takerTraits.skipMakerPermit()) {
_applyMakerPermit(order, orderHash, order.maker.get(), extension);
}
}

(makingAmount, takingAmount) = _fill(order, orderHash, remainingMakingAmount, amount, takerTraits, target, extension, interaction);
Expand Down Expand Up @@ -499,24 +504,6 @@ abstract contract OrderMixin is IOrderMixin, EIP712, OnlyWethReceiver, Predicate
if (remainingMakingAmount == 0) revert InvalidatedOrder();
}

/**
* @notice Executes the maker's permit and checks for potential reentrancy attacks.
* @param order The order to apply the permit for.
* @param orderHash The hash of the order.
* @param extension The extension data associated with the order.
*/
function _applyMakerPermit(IOrderMixin.Order calldata order, bytes32 orderHash, address maker, bytes calldata extension) private {
bytes calldata makerPermit = extension.makerPermit();
if (makerPermit.length >= 20) {
// proceed only if taker is willing to execute permit and its length is enough to store address
IERC20(address(bytes20(makerPermit))).tryPermit(maker, address(this), makerPermit[20:]);
if (!order.makerTraits.useBitInvalidator()) {
// Bit orders are not subjects for reentrancy, but we still need to check remaining-based orders for reentrancy
if (!_remainingInvalidator[order.maker.get()][orderHash].isNewOrder()) revert ReentrancyDetected();
}
}
}

/**
* @notice Calls the transferFrom function with an arbitrary suffix.
* @dev The suffix is appended to the end of the standard ERC20 transferFrom function parameters.
Expand Down

0 comments on commit db7fafd

Please sign in to comment.