Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(evm): C01 - Address incorrect use of relayerRefund over msg.sender in claimRelayerRefund function #826

Merged

Conversation

chrismaree
Copy link
Member

OZ identified the following issue in the contracts:

The claimRelayerRefund function is meant to give relayers the option to claim outstanding repayments. This can happen in different edge cases, like blacklists not allowing token transfers. In such cases, the relayer can call this function and specify a different refundAddress to claim their funds.

The function first reads the current outstanding refund from the relayerRefund mapping using the l2TokenAddress and msg.sender keys. If this value is greater than 0 then it is transferred to the refundAddress and the appropriate event is emitted.

Before transferring out the tokens, the mapping value is set to 0 for correct accounting. However, the key used to reset the mapping value is refundAddress and not msg.sender. This opens the door for any relayer with a small refund amount to zero out any other relayer's refund by specifying their refundAddress, effectively making the relayers lose their refunds. In addition, since the original mapping value is never reset, a malicious relayer can exploit this by repeatedly calling the function to drain the entire balance of the l2TokenAddress contract.

Consider using the proper msg.sender key instead of refundAddress to correctly set the refund amount to zero.

This PR addresses this by moving to using msg.sender within this method, as requested.

Signed-off-by: Chris Maree <[email protected]>
@chrismaree chrismaree changed the base branch from master to svm-dev January 2, 2025 10:28
@chrismaree chrismaree changed the title fix: C01: Address incorrect use of relayerrRefund over msg.sender in claimRelayerRefund function fix: C01 - Address incorrect use of relayerrRefund over msg.sender in claimRelayerRefund function Jan 2, 2025
@mrice32 mrice32 changed the title fix: C01 - Address incorrect use of relayerrRefund over msg.sender in claimRelayerRefund function fix: C01 - Address incorrect use of relayerRefund over msg.sender in claimRelayerRefund function Jan 6, 2025
Copy link
Contributor

@mrice32 mrice32 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@chrismaree chrismaree changed the title fix: C01 - Address incorrect use of relayerRefund over msg.sender in claimRelayerRefund function fix(evm): C01 - Address incorrect use of relayerRefund over msg.sender in claimRelayerRefund function Jan 7, 2025
@chrismaree chrismaree merged commit 155839e into svm-dev Jan 9, 2025
9 checks passed
@chrismaree chrismaree deleted the chrismaree/c-01-fix-non-claim-relaier-refund-address branch January 9, 2025 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants