sherlock-audit / 2024-09-symmio-v0-8-4-update-contest-judging

0 stars 0 forks source link

danyilbalko - The depositToReserveVault function must ensure arithmetic precision for the amount value. #36

Open sherlock-admin4 opened 1 week ago

sherlock-admin4 commented 1 week ago

danyilbalko

Medium

The depositToReserveVault function must ensure arithmetic precision for the amount value.

Summary

In the AccountFacetImpl.sol file,

the deposit function added a value to balances by ensuring arithmetic precision for the amount value,

but the depositToReserveVault function did not do so.

This can cause a loss of funds due to arithmetic errors in the system.

Root Cause

https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/facets/Account/AccountFacetImpl.sol#L119

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

No response

PoC

The system uses precision up to 18 decimal places for the balance value, so the arithmetic precision of the balances and reserveVault values ​​in the depositToReserveVault and withdrawalFromReserveVault functions must be the same. Otherwise, funds will be lost due to arithmetic errors.

Mitigation

function depositToReserveVault(uint256 amount, address partyB) internal {
    GlobalAppStorage.Layout storage appLayout = GlobalAppStorage.layout();
    AccountStorage.Layout storage accountLayout = AccountStorage.layout();
    require(amount <= accountLayout.balances[msg.sender], "AccountFacet: Insufficient balance");
    require(MAStorage.layout().partyBStatus[partyB], "AccountFacet: Should be partyB");
    uint256 amountWith18Decimals = (amount * 1e18) / (10 ** IERC20Metadata(appLayout.collateral).decimals());
    accountLayout.balances[msg.sender] -= amountWith18Decimals;
    accountLayout.reserveVault[partyB] += amountWith18Decimals;
}
ITStarMan100 commented 1 week ago

the code describes the amount as follows:

/// @param amount The exact amount of collateral to transfer to the emergency reserve, specified in 18 decimal places.

The fact that this is not verified and processed in the code means that the system is a security risk and could be exploited by a malicious user to lose funds.