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

Admin cannot collect protocol fee due to Incorrect Parameter Order in ERC20 Transfer Function Call #72

Closed
howlbot-integration bot opened this issue Sep 16, 2024 · 1 comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-84 🤖_38_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality

Comments

@howlbot-integration
Copy link

Lines of code

https://github.com/code-423n4/2024-08-superposition/blob/main/pkg/seawater/src/lib.rs#L1149-L1150

Vulnerability details

Impact

  • If the recipient address is not a valid token contract, the function will consistently revert so admin cannot collect the protocol fee.
  • If the recipient address is a valid token contract , The function calls transfer tokens from the wrong addresses (the recipient address instead of the intended token address).

Either way admin cannot collect the protocol fee

Proof of Concept

In the collect_protocol_7540_F_A_9_F function, there is a misuse of the erc20::transfer_to_addr function. The correct parameter order for this function is (token, recipient, amount)
https://github.com/code-423n4/2024-08-superposition/blob/main/pkg/seawater/src/wasm_erc20.rs#L158

but in the current implementation, the order is incorrect as it sending recipient in place of token Address .

erc20::transfer_to_addr(recipient, pool, U256::from(token_0))?;
erc20::transfer_to_addr(recipient, FUSDC_ADDR, U256::from(token_1))?;

Tools Used

Manual Review

Recommended Mitigation Steps

Correct the parameter order in the erc20::transfer_to_addr function calls:

erc20::transfer_to_addr(pool, recipient, U256::from(token_0))?;
erc20::transfer_to_addr(FUSDC_ADDR, recipient, U256::from(token_1))?;

Assessed type

Token-Transfer

@howlbot-integration howlbot-integration bot added 3 (High Risk) Assets can be stolen/lost/compromised directly 🤖_38_group AI based duplicate group recommendation bug Something isn't working duplicate-60 sufficient quality report This report is of sufficient quality labels Sep 16, 2024
howlbot-integration bot added a commit that referenced this issue Sep 16, 2024
@c4-judge
Copy link
Contributor

alex-ppg marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-84 🤖_38_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

1 participant