Skip to content

security: validate messageFrom to prevent identity spoofing#9

Open
paul-research wants to merge 1 commit into
circlefin:masterfrom
paul-research:security/validate-message-sender
Open

security: validate messageFrom to prevent identity spoofing#9
paul-research wants to merge 1 commit into
circlefin:masterfrom
paul-research:security/validate-message-sender

Conversation

@paul-research

Copy link
Copy Markdown

Validate messageFrom to prevent spoofing
Problem
messageFrom in ReciboInfo is not validated. Anyone can set it to an arbitrary address:

recibo.sendMsg(ReciboInfo({
messageFrom: PAUL_ADDRESS,
messageTo: victim,
message: encrypted_msg,
metadata: "{}"
}))
// Event shows "Paul sent message" but actual sender was attacker
This will get flagged in audit.

Solution
require(info.messageFrom == msg.sender, "Recibo: messageFrom must match sender");
Applied to all functions in both Recibo.sol and ReciboToken.sol.

Bonus: Added missing spender != address(this) check in permitWithMsg.

Potential pushback & responses
"We're adding AA soon, won't this block relayers?"
Yes, but that's the point. Current design has no signature verification - anyone can spoof.

When we add AA, we'll add separate functions:

sendMsg() - simple, direct calls (this PR)
sendMsgWithSignature() - supports relayers (Phase 2)
Both need to be secure. Can't ship AA with an insecure fallback function.

"But calldata has tx.from, so we can verify off-chain"
Events are for indexing. Not every UI/indexer will parse calldata for tx.from.
Attack vector: phishing notifications ("Circle sent you a message!").

Arc Chain = financial messaging with USDC gas = higher security bar.

"messageFrom is for flexibility"
Without signature verification, it's for spoofing.

AA requires EIP-712 signatures anyway. This PR doesn't prevent AA

Breaking change
Impact: Minimal. Python client already uses sender_address correctly.
Only affects hypothetical relayer code (which doesn't exist yet and would need signatures anyway).

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.
@osr21

osr21 commented Jul 15, 2026

Copy link
Copy Markdown

Memo indexer findings: messageFrom validation gap affects trust in off-chain memo search

We built a Transaction Memo Indexer (Tool B) for Arc Testnet that stores and queries Memo events from the predeployed contract at 0x5294E9927c3306DcBaDb03fe70b92e01cCede505. During indexing we noticed the same validation gap you've identified here.

In our indexer, we record the sender field from the Solidity event:

event Memo(address indexed sender, address indexed target, bytes32 callDataHash, bytes32 indexed memoId, bytes memo, uint256 memoIndex);

The sender in the event is the msg.sender of the memo() call — which is always the transaction originator and cannot be spoofed at the EVM level. That field is safe to trust.

However, if the memo's payload (the memo bytes field) contains an embedded messageFrom address (as Recibo's ReciboInfo struct does), anyone can forge it to impersonate another address in the off-chain memo content. Our indexer stores the raw hex and attempts UTF-8 decoding, but we mark decoded memo content as unverified display text — it should never be used for authentication decisions.

Recommendation: any dapp or indexer that reads structured memo payloads should:

  1. Trust event.sender (on-chain, unforgeable) for identity
  2. Treat the memo bytes payload as user-controlled display data — validate or ignore the embedded messageFrom field

The fix proposed in this issue (validating messageFrom == msg.sender on-chain) is the correct long-term solution for Recibo specifically. For the generic Arc Memo contract, that validation doesn't exist and consumers must handle it off-chain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants