fix(consensus): ConsensusMain.addTransaction trusts caller-supplied _sender over msg.sender

backend/protocol_rpc/transactions_parser.py

@@ -13,6 +13,7 @@
 from backend.rollup.consensus_service import ConsensusService
 from backend.rollup.web3_pool import Web3ConnectionPool
 from backend.domain.types import TransactionType
+from backend.errors.errors import InvalidTransactionError
 
 from backend.protocol_rpc.types import (
     DecodedDeploymentData,
@@ -597,6 +598,21 @@ def get_genlayer_transaction(
             )
 
         sender = rollup_transaction.data.args.sender
+        # `sender` above is an attacker-controlled argument decoded from the
+        # transaction's calldata, not the party that actually authorized the
+        # transaction. Only `rollup_transaction.from_address` is verified,
+        # by `transaction_has_valid_signature`, to be the recovered signer
+        # of the raw transaction. Without this check, a caller can sign a
+        # valid transaction from their own address while setting `sender`
+        # to any other address, which becomes the Intelligent Contract
+        # caller (see GenVM's `gl.message.sender_address`) while the real
+        # signer merely pays the gas.
+        if to_checksum_address(sender) != to_checksum_address(
+            rollup_transaction.from_address
+        ):
+            raise InvalidTransactionError(
+                "Transaction sender argument does not match the recovered signer"
+            )
         # The v0.6 deploySalted selector is authoritative: Consensus ignores
         # the tuple recipient and always passes address(0) into CreationPhase.
         # Inferring only from the user-controlled recipient let a salted deploy

hardhat/contracts/v2_contracts/ConsensusMain.sol

@@ -303,18 +303,29 @@ contract ConsensusMain is
 		uint256 _maxRotations,
 		bytes memory _txData
 	) external {
-		if (_txData.length > 0) {
-			_addTransaction(
-				_sender,
-				_recipient,
-				_numOfInitialValidators,
-				_maxRotations,
-				0,
-				_txData
-			);
-		} else {
+		if (_txData.length == 0) {
 			revert Errors.EmptyTransaction();
 		}
+		// A caller may only submit a transaction on behalf of another
+		// address if it is a registered ghost contract relaying a call for
+		// its own caller (see GhostBlueprint.addTransaction). Otherwise
+		// `_sender` must be the caller itself, or left unset (address(0),
+		// defaulted to msg.sender below).
+		if (
+			_sender != address(0) &&
+			_sender != msg.sender &&
+			!ghostContracts[msg.sender]
+		) {
+			revert Errors.UnauthorizedSender();
+		}
+		_addTransaction(
+			_sender,
+			_recipient,
+			_numOfInitialValidators,
+			_maxRotations,
+			0,
+			_txData
+		);
 		// TODO: Fee verification handling
 	}
 
@@ -396,8 +407,18 @@ contract ConsensusMain is
 		} else {
 			saltNonce = 0;
 		}
+		address feeAwareSender = address(uint160(senderWord));
+		// Same authorization rule as the plain `addTransaction` entrypoint:
+		// see the comment there for why `_sender` cannot be trusted as-is.
+		if (
+			feeAwareSender != address(0) &&
+			feeAwareSender != msg.sender &&
+			!ghostContracts[msg.sender]
+		) {
+			revert Errors.UnauthorizedSender();
+		}
 		_addTransaction(
-			address(uint160(senderWord)),
+			feeAwareSender,
 			address(uint160(recipientWord)),
 			numOfInitialValidators,
 			maxRotations,

hardhat/contracts/v2_contracts/ConsensusMainWithFees.sol

@@ -75,6 +75,18 @@ contract ConsensusMainWithFees is
 		if (_txData.length == 0) {
 			revert Errors.EmptyTransaction();
 		}
+		// A caller may only submit a transaction on behalf of another
+		// address if it is a registered ghost contract relaying a call for
+		// its own caller (see GhostBlueprint.addTransaction). Otherwise
+		// `_sender` must be the caller itself, or left unset (address(0),
+		// defaulted to msg.sender below).
+		if (
+			_sender != address(0) &&
+			_sender != msg.sender &&
+			!ghostContracts[msg.sender]
+		) {
+			revert Errors.UnauthorizedSender();
+		}
 		uint totalFeesToPay = feeManager.calculateRoundFees(
 			bytes32(0),
 			_feesDistribution,

hardhat/contracts/v2_contracts/utils/Errors.sol

@@ -85,4 +85,5 @@ library Errors {
 	error CallerNotModule();
 	error CallerNotOwner();
 	error InvalidProcessingBlock();
+	error UnauthorizedSender();
 }

tests/unit/test_transactions_parser.py

@@ -2,6 +2,8 @@
 from unittest.mock import Mock, MagicMock
 from backend.database_handler.transactions_processor import TransactionsProcessor
 from backend.database_handler.models import TransactionStatus
+from backend.domain.types import TransactionType
+from backend.errors.errors import InvalidTransactionError
 from backend.protocol_rpc.transactions_parser import (
     TransactionParser,
     DecodedMethodSendData,
@@ -13,7 +15,6 @@
     DecodedRollupTransactionDataArgs,
     ZERO_ADDRESS,
 )
-from backend.domain.types import TransactionType
 import re
 from typing import Optional, List, Any
 from rlp import encode
@@ -1070,3 +1071,61 @@ def test_pre_fee_submission_decodes_without_a_rollup_connection(monkeypatch):
     genlayer_transaction = parser.get_genlayer_transaction(decoded)
     assert genlayer_transaction.type == TransactionType.DEPLOY_CONTRACT
     assert genlayer_transaction.data.contract_code == contract_code
+
+
+def _rollup_transaction_with_sender(
+    *, from_address: str, args_sender: str
+) -> DecodedRollupTransaction:
+    return DecodedRollupTransaction(
+        from_address=from_address,
+        to_address="0x6666666666666666666666666666666666666666",
+        data=DecodedRollupTransactionData(
+            function_name="addTransaction",
+            args=DecodedRollupTransactionDataArgs(
+                sender=args_sender,
+                recipient="0x6666666666666666666666666666666666666666",
+                num_of_initial_validators=5,
+                max_rotations=3,
+                data=encode([b"\x00"]),
+            ),
+        ),
+        type=2,
+        nonce=1,
+        value=0,
+    )
+
+
+def test_get_genlayer_transaction_uses_recovered_signer_when_sender_matches(
+    transaction_parser,
+):
+    # Legitimate case: the caller signed for themselves, so the `sender`
+    # argument and the recovered signer agree.
+    rollup_transaction = _rollup_transaction_with_sender(
+        from_address="0x7777777777777777777777777777777777777777",
+        args_sender="0x7777777777777777777777777777777777777777",
+    )
+
+    genlayer_transaction = transaction_parser.get_genlayer_transaction(
+        rollup_transaction
+    )
+
+    assert (
+        genlayer_transaction.from_address
+        == "0x7777777777777777777777777777777777777777"
+    )
+    assert genlayer_transaction.type == TransactionType.RUN_CONTRACT
+
+
+def test_get_genlayer_transaction_rejects_sender_spoofing(transaction_parser):
+    # Attacker signs the transaction from their own address (from_address)
+    # but sets the `sender` calldata argument to a victim address. Trusting
+    # `sender` here would let the attacker impersonate the victim as the
+    # Intelligent Contract caller while the victim never authorized
+    # anything - the real signer merely pays the gas.
+    rollup_transaction = _rollup_transaction_with_sender(
+        from_address="0x8888888888888888888888888888888888888888",  # attacker
+        args_sender="0x9999999999999999999999999999999999999999",  # victim
+    )
+
+    with pytest.raises(InvalidTransactionError):
+        transaction_parser.get_genlayer_transaction(rollup_transaction)