From 239bb134a794428334246d24beac9f1afcb87bf6 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Tue, 25 Jul 2023 14:32:32 -0600 Subject: [PATCH 1/9] Make ERC2771Context return original sender address if `msg.data.length <= 20` --- contracts/metatx/ERC2771Context.sol | 2 +- test/metatx/ERC2771Context.test.js | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/contracts/metatx/ERC2771Context.sol b/contracts/metatx/ERC2771Context.sol index 35de9f7ba73..9ade6148d39 100644 --- a/contracts/metatx/ERC2771Context.sol +++ b/contracts/metatx/ERC2771Context.sol @@ -22,7 +22,7 @@ abstract contract ERC2771Context is Context { } function _msgSender() internal view virtual override returns (address sender) { - if (isTrustedForwarder(msg.sender)) { + if (isTrustedForwarder(msg.sender) && msg.data.length >= 20) { // The assembly code is more direct than the Solidity version using `abi.decode`. /// @solidity memory-safe-assembly assembly { diff --git a/test/metatx/ERC2771Context.test.js b/test/metatx/ERC2771Context.test.js index 3dd4b41532d..f30dc8446fa 100644 --- a/test/metatx/ERC2771Context.test.js +++ b/test/metatx/ERC2771Context.test.js @@ -12,6 +12,8 @@ const ContextMockCaller = artifacts.require('ContextMockCaller'); const { shouldBehaveLikeRegularContext } = require('../utils/Context.behavior'); contract('ERC2771Context', function (accounts) { + const [, anotherAccount] = accounts; + const MAX_UINT48 = web3.utils.toBN(1).shln(48).subn(1).toString(); beforeEach(async function () { @@ -79,6 +81,15 @@ contract('ERC2771Context', function (accounts) { const { tx } = await this.forwarder.execute(req); await expectEvent.inTransaction(tx, ERC2771ContextMock, 'Sender', { sender: this.sender }); }); + + it('returns the original sender when calldata length is less than 20 bytes (address length)', async function () { + // The forwarder doesn't produce calls with calldata length less than 20 bytes + const recipient = await ERC2771ContextMock.new(anotherAccount); + + const { tx } = await recipient.msgSender({ from: anotherAccount }); + + await expectEvent.inTransaction(tx, ERC2771ContextMock, 'Sender', { sender: anotherAccount }); + }); }); describe('msgData', function () { From ae34a9cf041a83fe183831d84c6987dc5b4eb1c3 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Tue, 25 Jul 2023 14:36:42 -0600 Subject: [PATCH 2/9] Lint --- test/metatx/ERC2771Context.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/metatx/ERC2771Context.test.js b/test/metatx/ERC2771Context.test.js index f30dc8446fa..061e765bdaa 100644 --- a/test/metatx/ERC2771Context.test.js +++ b/test/metatx/ERC2771Context.test.js @@ -81,13 +81,13 @@ contract('ERC2771Context', function (accounts) { const { tx } = await this.forwarder.execute(req); await expectEvent.inTransaction(tx, ERC2771ContextMock, 'Sender', { sender: this.sender }); }); - + it('returns the original sender when calldata length is less than 20 bytes (address length)', async function () { // The forwarder doesn't produce calls with calldata length less than 20 bytes const recipient = await ERC2771ContextMock.new(anotherAccount); - + const { tx } = await recipient.msgSender({ from: anotherAccount }); - + await expectEvent.inTransaction(tx, ERC2771ContextMock, 'Sender', { sender: anotherAccount }); }); }); From f133d09616674af69d13399d3780fc4321ec1cee Mon Sep 17 00:00:00 2001 From: ernestognw Date: Tue, 25 Jul 2023 14:40:12 -0600 Subject: [PATCH 3/9] Add Changeset --- .changeset/unlucky-beans-obey.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/unlucky-beans-obey.md diff --git a/.changeset/unlucky-beans-obey.md b/.changeset/unlucky-beans-obey.md new file mode 100644 index 00000000000..8f8906116fb --- /dev/null +++ b/.changeset/unlucky-beans-obey.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': patch +--- + +`ERC2771Context`: Returns the forwarder address whenever `msg.data.length` is not appended with an address (i.e. length is less than 20 bytes). From 9dc13316310b20874cc406248b7d16234f1d2d4f Mon Sep 17 00:00:00 2001 From: ernestognw Date: Tue, 25 Jul 2023 14:41:27 -0600 Subject: [PATCH 4/9] Adjust changeset --- .changeset/unlucky-beans-obey.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/unlucky-beans-obey.md b/.changeset/unlucky-beans-obey.md index 8f8906116fb..a481d73745d 100644 --- a/.changeset/unlucky-beans-obey.md +++ b/.changeset/unlucky-beans-obey.md @@ -2,4 +2,4 @@ 'openzeppelin-solidity': patch --- -`ERC2771Context`: Returns the forwarder address whenever `msg.data.length` is not appended with an address (i.e. length is less than 20 bytes). +`ERC2771Context`: Return the forwarder address whenever `msg.data.length` is not appended with the request signer address (i.e. length is less than 20 bytes). From fb35cf603f561580154bbee5d6effe7a2012ad9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= Date: Tue, 25 Jul 2023 14:51:51 -0600 Subject: [PATCH 5/9] Update .changeset/unlucky-beans-obey.md Co-authored-by: Hadrien Croubois --- .changeset/unlucky-beans-obey.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/unlucky-beans-obey.md b/.changeset/unlucky-beans-obey.md index a481d73745d..edbdce2ad9f 100644 --- a/.changeset/unlucky-beans-obey.md +++ b/.changeset/unlucky-beans-obey.md @@ -2,4 +2,4 @@ 'openzeppelin-solidity': patch --- -`ERC2771Context`: Return the forwarder address whenever `msg.data.length` is not appended with the request signer address (i.e. length is less than 20 bytes). +`ERC2771Context`: Return the forwarder address whenever the `msg.data` or calls sent by a trusted forwarder is not long enough to contain the request signer address (i.e. `msg.data.length` is less than 20 bytes). From 1d035e2676df7c5b80dc68128b228282a8c325d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= Date: Tue, 25 Jul 2023 14:55:19 -0600 Subject: [PATCH 6/9] Update test/metatx/ERC2771Context.test.js Co-authored-by: Hadrien Croubois --- test/metatx/ERC2771Context.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/metatx/ERC2771Context.test.js b/test/metatx/ERC2771Context.test.js index 061e765bdaa..a907e502cdd 100644 --- a/test/metatx/ERC2771Context.test.js +++ b/test/metatx/ERC2771Context.test.js @@ -86,9 +86,9 @@ contract('ERC2771Context', function (accounts) { // The forwarder doesn't produce calls with calldata length less than 20 bytes const recipient = await ERC2771ContextMock.new(anotherAccount); - const { tx } = await recipient.msgSender({ from: anotherAccount }); + const { receipt } = await recipient.msgSender({ from: anotherAccount }); - await expectEvent.inTransaction(tx, ERC2771ContextMock, 'Sender', { sender: anotherAccount }); + await expectEvent(receipt, 'Sender', { sender: anotherAccount }); }); }); From dd1ef0c75982a7e8194d84a415d013250575a83e Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 25 Jul 2023 23:02:35 +0200 Subject: [PATCH 7/9] Update unlucky-beans-obey.md --- .changeset/unlucky-beans-obey.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/unlucky-beans-obey.md b/.changeset/unlucky-beans-obey.md index edbdce2ad9f..e099340b682 100644 --- a/.changeset/unlucky-beans-obey.md +++ b/.changeset/unlucky-beans-obey.md @@ -2,4 +2,4 @@ 'openzeppelin-solidity': patch --- -`ERC2771Context`: Return the forwarder address whenever the `msg.data` or calls sent by a trusted forwarder is not long enough to contain the request signer address (i.e. `msg.data.length` is less than 20 bytes). +`ERC2771Context`: Return the forwarder address whenever the `msg.data` of a calls originating from a trusted forwarder is not long enough to contain the request signer address (i.e. `msg.data.length` is less than 20 bytes). From 51c190c2672003eb73f0de94614da7ba94ec605d Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 25 Jul 2023 23:02:56 +0200 Subject: [PATCH 8/9] Update unlucky-beans-obey.md --- .changeset/unlucky-beans-obey.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/unlucky-beans-obey.md b/.changeset/unlucky-beans-obey.md index e099340b682..c679ad019ee 100644 --- a/.changeset/unlucky-beans-obey.md +++ b/.changeset/unlucky-beans-obey.md @@ -2,4 +2,4 @@ 'openzeppelin-solidity': patch --- -`ERC2771Context`: Return the forwarder address whenever the `msg.data` of a calls originating from a trusted forwarder is not long enough to contain the request signer address (i.e. `msg.data.length` is less than 20 bytes). +`ERC2771Context`: Return the forwarder address whenever the `msg.data` of a call originating from a trusted forwarder is not long enough to contain the request signer address (i.e. `msg.data.length` is less than 20 bytes). From f056ce5adcc5690c91ca5ca3c918d1dd5dfe8314 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= Date: Tue, 25 Jul 2023 15:30:58 -0600 Subject: [PATCH 9/9] Update .changeset/unlucky-beans-obey.md Co-authored-by: Francisco --- .changeset/unlucky-beans-obey.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/unlucky-beans-obey.md b/.changeset/unlucky-beans-obey.md index c679ad019ee..e472d3c6cb6 100644 --- a/.changeset/unlucky-beans-obey.md +++ b/.changeset/unlucky-beans-obey.md @@ -2,4 +2,4 @@ 'openzeppelin-solidity': patch --- -`ERC2771Context`: Return the forwarder address whenever the `msg.data` of a call originating from a trusted forwarder is not long enough to contain the request signer address (i.e. `msg.data.length` is less than 20 bytes). +`ERC2771Context`: Return the forwarder address whenever the `msg.data` of a call originating from a trusted forwarder is not long enough to contain the request signer address (i.e. `msg.data.length` is less than 20 bytes), as specified by ERC-2771.