hats-finance / Most--Aleph-Zero-Bridge-0xab7c1d45ae21e7133574746b2985c58e0ae2e61d

Aleph Zero bridge to Ethereum
Apache License 2.0
0 stars 1 forks source link

decrease in pocket_money_balance even if the transfer failed #21

Open hats-bug-reporter[bot] opened 4 months ago

hats-bug-reporter[bot] commented 4 months ago

Github username: @0xmahdirostami Twitter username: 0xmahdirostami Submission hash (on-chain): 0xd5bf26ded8721aae73504af9b0b8a4d0e1976659e20a492b2c94b902d229a3c4 Severity: high

Description: Description In the receive_request function, there's a section of code where a transfer is attempted, and if it fails, it doesn't revert but still decrements pocket_money_balance.

                if data.pocket_money_balance >= data.pocket_money {
                    // don't revert if the transfer fails
                    _ = self
                        .env()
                        .transfer(dest_receiver_address.into(), data.pocket_money);
                    data.pocket_money_balance = data
                        .pocket_money_balance
                        .checked_sub(data.pocket_money)
                        .ok_or(MostError::Arithmetic)?;
                }

Impact
This behavior is inconsistent and could lead to incorrect state management. If the transfer fails, the pocket_money_balance should not be decremented. pocket_money_balance should remain same

krzysztofziobro commented 4 months ago

Invalid submission: A PoC is required for submission to be considered valid. You can create a new submission that contains a working PoC.

0xmahdirostami commented 3 months ago

@krzysztofziobro

hello sir, the issue is mitigated in https://github.com/Cardinal-Cryptography/most/pull/164/files which means my reported issue was valid.

The issue is considered invalid due to lack of proof of concept (POC), but as sponsors mentioned it is not necessary for low and minor issues.

Also, the severity of the issue is not high, it should still be considered a valid low or minor issue.

as mentioned in https://hatsfinance.medium.com/aleph-zero-bridge-audit-competition-rewards-up-to-32k-in-usdt-7015b4ab09cf this issue must be valid as low. Low severity:

fonstack commented 3 months ago

Changing to minor after dispute resolution. Congrats @0xmahdirostami