From 28d9ac2bdb321b24fe06b8b916ee2962889f772b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= <ernestognw@gmail.com>
Date: Tue, 25 Jul 2023 15:48:23 -0600
Subject: [PATCH] Make ERC2771Context return original sender address if
 `msg.data.length <= 20` (#4481)

---
 .changeset/unlucky-beans-obey.md    |  5 +++++
 contracts/metatx/ERC2771Context.sol |  2 +-
 test/metatx/ERC2771Context.test.js  | 11 +++++++++++
 3 files changed, 17 insertions(+), 1 deletion(-)
 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..e472d3c6cb6
--- /dev/null
+++ b/.changeset/unlucky-beans-obey.md
@@ -0,0 +1,5 @@
+---
+'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), as specified by ERC-2771.
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..a907e502cdd 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 { receipt } = await recipient.msgSender({ from: anotherAccount });
+
+        await expectEvent(receipt, 'Sender', { sender: anotherAccount });
+      });
     });
 
     describe('msgData', function () {