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 (audit AUR-11): Infinite Amounts Can Be Transferred to One’s Self #15

Merged
merged 2 commits into from
Feb 10, 2023

Conversation

mrLSD
Copy link
Collaborator

@mrLSD mrLSD commented Feb 8, 2023

Description

After Sigma Prime Aurora Engine and Eth-Connector audit, was detected potential issue: AUR-11 - Infinite Amounts Can Be Transferred to One’s.

Details

Balance checks are performed in the function internal_transfer_eth_on_near(). The following snippet of the function ft_transfer_call() show how the balance checks are skipped if sender_id == receiver_id.

if sender_id != receiver_id {
    self.internal_transfer_eth_on_near(&sender_id, &receiver_id, amount, memo)?;
}

The impact is not significant to the current contract as the net balances and total supply remain unchanged. However, this poses a potential threat to third party contracts. ft_transfer_call() makes the external call receiver_id.ft_on_transfer(amount, msg, sender_id). Since we are able to set amount as any arbitrary value this
may lead to issues in the accounting of third party contracts.

Solution

Added extra verification in ft_transfer_call, for the case sender_id == receiver_id:

  • amount > 0
  • balance_of_sender >= amount

Gas cost

Not changed

How to review

Pay attention to ft_transfer_call method

Tests

Extended to check the balance for sender_id, when sender_id == receiver_id in ft_transfer_call.

@mrLSD mrLSD self-assigned this Feb 8, 2023
@mrLSD mrLSD requested review from birchmd and joshuajbouw February 8, 2023 23:19
@mrLSD mrLSD added this to the v0.4.0 milestone Feb 8, 2023
Copy link

@joshuajbouw joshuajbouw left a comment

Choose a reason for hiding this comment

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

Looks like this settles that issue. Thanks.

@mrLSD mrLSD merged commit 39b1d83 into master Feb 10, 2023
@mrLSD mrLSD deleted the fix/audit-aur-11 branch February 27, 2023 22:47
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.

3 participants