Address courier front-running and persistent allowances (#216)

core/src/Lender.sol

@@ -107,18 +107,24 @@ contract Lender is Ledger {
      * supports the additional flow where you prepay `amount` instead of relying on approve/transferFrom.
      * @param amount The amount of underlying tokens to deposit
      * @param beneficiary The receiver of `shares`
-     * @param courierId The identifier of the referrer to credit for this deposit. 0 indicates none.
+     * @param courierId The ID of the courier (or 0, to indicate lack thereof) that will receive a cut of
+     * `beneficiary`'s future interest. Only takes effect when `balanceOf(beneficiary) == 0`. In
+     * all other cases, pass 0 to avoid wasting gas on courier-related checks.
      * @return shares The number of shares (banknotes) minted to `beneficiary`
      */
     function deposit(uint256 amount, address beneficiary, uint32 courierId) public returns (uint256 shares) {
         if (courierId != 0) {
+            // Callers are free to set their own courier, but they need permission to mess with others'
+            if (msg.sender != beneficiary) {
+                require(allowance[beneficiary][msg.sender] > 0, "Aloe: courier");
+                allowance[beneficiary][msg.sender] = 0;
+            }
+
             (address courier, uint16 cut) = FACTORY.couriers(courierId);
 
             require(
-                // Callers are free to set their own courier, but they need permission to mess with others'
-                (msg.sender == beneficiary || allowance[beneficiary][msg.sender] != 0) &&
-                    // Prevent `RESERVE` from having a courier, since its principle wouldn't be tracked properly
-                    (beneficiary != RESERVE) &&
+                // Prevent `RESERVE` from having a courier, since its principle wouldn't be tracked properly
+                (beneficiary != RESERVE) &&
                     // Payout logic can't handle self-reference, so don't let accounts credit themselves
                     (beneficiary != courier) &&
                     // Make sure `cut` has been set

periphery/src/Router.sol

@@ -16,24 +16,49 @@ contract Router {
         PERMIT2 = permit2;
     }
 
-    function depositWithPermit(
+    /**
+     * @notice Deposits `amount` of `lender.asset()` to `lender` using {`nonce`, `deadline`, `signature`} for Permit2,
+     * and gives `courierId` a cut of future interest earned by `msg.sender`. `v`, `r`, and `s` are used with
+     * `lender.permit` in order to (a) achieve 0 balance if necessary and (b) set the courier.
+     * @dev This innoculates `Lender` against a potential courier frontrunning attack by redeeming all shares (if any
+     * are present) before assigning the new `courierId`. `Lender` then clears the `permit`ed allowance in `deposit`,
+     * meaning this contract is left with no special permissions.
+     */
+    function depositWithPermit2(
         Lender lender,
         uint256 amount,
-        uint256 allowance,
+        uint256 nonce,
         uint256 deadline,
+        bytes calldata signature,
+        uint32 courierId,
         uint8 v,
         bytes32 r,
-        bytes32 s,
-        uint32 courierId,
-        uint8 vL,
-        bytes32 rL,
-        bytes32 sL
+        bytes32 s
     ) external returns (uint256 shares) {
-        lender.permit(msg.sender, address(this), 1, deadline, vL, rL, sL);
+        lender.permit(msg.sender, address(this), type(uint256).max, deadline, v, r, s);
+
+        if (lender.balanceOf(msg.sender) > 0) {
+            lender.redeem(type(uint256).max, msg.sender, msg.sender);
+        }
 
-        ERC20 asset = lender.asset();
-        asset.permit(msg.sender, address(this), allowance, deadline, v, r, s);
-        asset.safeTransferFrom(msg.sender, address(lender), amount);
+        // Transfer tokens from the caller to the lender.
+        PERMIT2.permitTransferFrom(
+            // The permit message.
+            IPermit2.PermitTransferFrom({
+                permitted: IPermit2.TokenPermissions({token: lender.asset(), amount: amount}),
+                nonce: nonce,
+                deadline: deadline
+            }),
+            // The transfer recipient and amount.
+            IPermit2.SignatureTransferDetails({to: address(lender), requestedAmount: amount}),
+            // The owner of the tokens, which must also be
+            // the signer of the message, otherwise this call
+            // will fail.
+            msg.sender,
+            // The packed signature that was the result of signing
+            // the EIP712 hash of `permit`.
+            signature
+        );
 
         shares = lender.deposit(amount, msg.sender, courierId);
     }