From 7033acffc120767defba7df1e991ab10f7abca33 Mon Sep 17 00:00:00 2001 From: Paul Kwon Date: Tue, 18 Nov 2025 03:11:15 -0800 Subject: [PATCH] security: validate messageFrom to prevent identity spoofing Require messageFrom matches actual sender/owner to prevent users from impersonating others in message events. - Add validation in Recibo.sol (5 functions) and ReciboToken.sol (3 functions) - Fix missing spender check in permitWithMsg - Add error messages to all require statements - Update test fixtures accordingly All 103 tests pass. Gas increase: ~500 per call. --- src/Recibo.sol | 8 +++++++- src/ReciboToken.sol | 3 +++ test/Recibo.t.sol | 13 ++++++++++--- test/ReciboGasTest.t.sol | 3 ++- 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/Recibo.sol b/src/Recibo.sol index 31a82e1..797e461 100644 --- a/src/Recibo.sol +++ b/src/Recibo.sol @@ -52,6 +52,7 @@ contract Recibo is ReciboEvents { function sendMsg( ReciboInfo calldata info ) public { + require(info.messageFrom == msg.sender, "Recibo: messageFrom must match sender"); emit SentMsg(msg.sender, info.messageFrom, info.messageTo); } @@ -67,6 +68,7 @@ contract Recibo is ReciboEvents { uint256 value, ReciboInfo calldata info ) public returns (bool) { + require(info.messageFrom == msg.sender, "Recibo: messageFrom must match sender"); emit TransferWithMsg(msg.sender, to, info.messageFrom, info.messageTo, value); return _token.transferFrom(msg.sender, to, value); } @@ -94,7 +96,9 @@ contract Recibo is ReciboEvents { bytes32 s, ReciboInfo calldata info ) public { - require(owner != address(this)); + require(owner != address(this), "Recibo: owner cannot be this contract"); + require(spender != address(this), "Recibo: spender cannot be this contract"); + require(info.messageFrom == owner, "Recibo: messageFrom must match owner"); emit ApproveWithMsg(owner, spender, info.messageFrom, info.messageTo, value); _token.permit(owner, spender, value, deadline, v, r, s); } @@ -121,6 +125,7 @@ contract Recibo is ReciboEvents { bytes32 s, ReciboInfo calldata info ) public returns (bool) { + require(info.messageFrom == msg.sender, "Recibo: messageFrom must match sender"); emit TransferWithMsg(msg.sender, to, info.messageFrom, info.messageTo, value); _token.permit(msg.sender, address(this), value, deadline, v, r, s); return _token.transferFrom(msg.sender, to, value); @@ -148,6 +153,7 @@ contract Recibo is ReciboEvents { bytes memory signature, ReciboInfo calldata info ) public { + require(info.messageFrom == from, "Recibo: messageFrom must match from"); emit TransferWithMsg(from, to, info.messageFrom, info.messageTo, value); _token.transferWithAuthorization(from, to, value, validAfter, validBefore, nonce, signature); } diff --git a/src/ReciboToken.sol b/src/ReciboToken.sol index 00894a0..c728749 100644 --- a/src/ReciboToken.sol +++ b/src/ReciboToken.sol @@ -42,6 +42,7 @@ contract ReciboToken is ERC20, ReciboEvents { * @dev See {ERC20-transfer}. */ function transferWithMsg(address to, uint256 value, address messageFrom, address messageTo, string calldata metadata, bytes calldata message) public returns (bool) { + require(messageFrom == msg.sender, "ReciboToken: messageFrom must match sender"); emit TransferWithMsg(msg.sender, to, messageFrom, messageTo, value); return transfer(to, value); } @@ -51,6 +52,7 @@ contract ReciboToken is ERC20, ReciboEvents { * @dev See {ERC20-approve}. */ function approveWithMsg(address spender, uint256 value, address messageFrom, address messageTo, string calldata metadata, bytes calldata message) public returns (bool) { + require(messageFrom == msg.sender, "ReciboToken: messageFrom must match sender"); emit ApproveWithMsg(msg.sender, spender, messageFrom, messageTo, value); return approve(spender, value); } @@ -60,6 +62,7 @@ contract ReciboToken is ERC20, ReciboEvents { * @dev See {ERC20-transferFrom}. */ function transferFromWithMsg(address from, address to, uint256 value, address messageFrom, address messageTo, string calldata metadata, bytes calldata message) public returns (bool) { + require(messageFrom == from, "ReciboToken: messageFrom must match from"); emit TransferWithMsg(from, to, messageFrom, messageTo, value); return transferFrom(from, to, value); } diff --git a/test/Recibo.t.sol b/test/Recibo.t.sol index 553eaad..b8ad005 100644 --- a/test/Recibo.t.sol +++ b/test/Recibo.t.sol @@ -32,7 +32,7 @@ contract ReciboExtensionTest is GaslessTestBase { address private minter; uint256 private minterKey; bytes private msgBytes = abi.encode("message"); - Recibo.ReciboInfo private info = Recibo.ReciboInfo(minter, user, "metadata", msgBytes); + Recibo.ReciboInfo private info; uint private deadline; uint256 private validBefore; @@ -40,6 +40,7 @@ contract ReciboExtensionTest is GaslessTestBase { function setUp() public { (minter, minterKey) = makeAddrAndKey("minter"); + info = Recibo.ReciboInfo(minter, user, "metadata", msgBytes); vm.startPrank(minter); token = new GaslessToken("Token", "TKN", 200); @@ -52,9 +53,12 @@ contract ReciboExtensionTest is GaslessTestBase { function test_sendMsg() public { + // Need to call from minter since info.messageFrom == minter + vm.startPrank(minter); vm.expectEmit(true, true, true, true); - emit ReciboEvents.SentMsg(address(this), info.messageFrom, info.messageTo); + emit ReciboEvents.SentMsg(minter, info.messageFrom, info.messageTo); recibo.sendMsg(info); + vm.stopPrank(); } function test_transferWithMsg() public { @@ -92,10 +96,13 @@ contract ReciboExtensionTest is GaslessTestBase { bytes32 permit = buildPermitMessage(minter, address(recibo), 40, deadline, token); (uint8 v, bytes32 r, bytes32 s) = vm.sign(minterKey, permit); + // Create info with user as messageFrom to pass validation + Recibo.ReciboInfo memory userInfo = Recibo.ReciboInfo(user, user, "metadata", msgBytes); + vm.startPrank(user); address fakeSigner = 0x4816Daec8E87b4ebCd4f44e6A16c7019aeFA1150; vm.expectRevert(abi.encodeWithSelector(ERC20Permit.ERC2612InvalidSigner.selector, fakeSigner, user)); - recibo.permitAndTransferFromWithMsg(user, value, deadline, v, r, s, info); + recibo.permitAndTransferFromWithMsg(user, value, deadline, v, r, s, userInfo); vm.stopPrank(); } diff --git a/test/ReciboGasTest.t.sol b/test/ReciboGasTest.t.sol index 854f79f..85df785 100644 --- a/test/ReciboGasTest.t.sol +++ b/test/ReciboGasTest.t.sol @@ -33,7 +33,7 @@ contract ReciboGasTest is GaslessTestBase, BigMsg { address private minter; uint256 private minterKey; bytes private msgBytes = abi.encode("message"); - Recibo.ReciboInfo private info = Recibo.ReciboInfo(minter, user, "metadata", msgBytes); + Recibo.ReciboInfo private info; uint private deadline; uint256 private validBefore; @@ -41,6 +41,7 @@ contract ReciboGasTest is GaslessTestBase, BigMsg { function setUp() public { (minter, minterKey) = makeAddrAndKey("minter"); + info = Recibo.ReciboInfo(minter, user, "metadata", msgBytes); deadline = makeDeadline(10); (validAfter, validBefore) = makeValidTime(100);