From bf14518b6ddae350fcdc768798bd020677e52fd0 Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Tue, 28 Apr 2026 22:09:45 +0400 Subject: [PATCH] harmonise from-allowed check across LibFlow.flowERC20/721/1155 flowERC20 used an if/else if/else branch for the from-allowed check while flowERC721 and flowERC1155 used a !=-predicate gate. The divergence forced a reader to reverify the same security-critical invariant in two different shapes. flowERC20 now matches the !=-predicate style. The OZ-allowance branch that selects safeTransfer vs safeTransferFrom is kept separately, with a comment explaining why self-flow needs safeTransfer (transferFrom consumes allowance from the caller, even when caller == self). Closes #304. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/lib/LibFlow.sol | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/lib/LibFlow.sol b/src/lib/LibFlow.sol index ce4c0aa3..ac58ce07 100644 --- a/src/lib/LibFlow.sol +++ b/src/lib/LibFlow.sol @@ -78,14 +78,18 @@ library LibFlow { ERC20Transfer memory transfer; for (uint256 i = 0; i < flowTransfer.erc20.length; i++) { transfer = flowTransfer.erc20[i]; + // We don't support `from` as anyone other than `you` or `me` + // as this would allow for all kinds of issues re: approvals. + if (transfer.from != msg.sender && transfer.from != address(this)) { + revert UnsupportedERC20Flow(); + } + // OZ ERC20 `transferFrom` consumes allowance even when + // `from == msg.sender`, so the self-flow branch must use + // `safeTransfer` instead. if (transfer.from == msg.sender) { IERC20(transfer.token).safeTransferFrom(msg.sender, transfer.to, transfer.amount); - } else if (transfer.from == address(this)) { - IERC20(transfer.token).safeTransfer(transfer.to, transfer.amount); } else { - // We don't support `from` as anyone other than `you` or `me` - // as this would allow for all kinds of issues re: approvals. - revert UnsupportedERC20Flow(); + IERC20(transfer.token).safeTransfer(transfer.to, transfer.amount); } } }