Omniscia Gravita Protocol Audit

SafetyTransfer Manual Review Findings

SafetyTransfer Manual Review Findings

STR-01M: Incorrect Decimal Assumption

TypeSeverityLocation
Logical FaultSafetyTransfer.sol:L21

Description:

The SafetyTransfer::decimalsCorrection function will misbehave if the _token has decimals that are greater than the value of 18.

Impact:

The decimal correction mechanism will be incorrect in tokens with abnormal decimals, yielding significantly less values than expected.

Example:

contracts/Dependencies/SafetyTransfer.sol
11//_amount is in ether (1e18) and we want to convert it to the token decimal
12function decimalsCorrection(address _token, uint256 _amount)
13 internal
14 view
15 returns (uint256)
16{
17 if (_token == address(0)) return _amount;
18 if (_amount == 0) return 0;
19
20 uint8 decimals = ERC20Decimals(_token).decimals();
21 if (decimals < 18) {
22 return _amount.div(10**(18 - decimals));
23 }
24
25 return _amount;
26}

Recommendation:

We advise the code to introduce an else branch that evaluates whether decimals is greater-than (>) the value of 18, in which case it should offset the _amount via a multiplication rather than division.

Alleviation:

The decimals of a token are properly handled by the SafetyTransfer::decimalsCorrection function as they are normalized in either an upwards or downwards trajectory depending on whether the decimals exceed the default value of 18 or subceed it.

STR-02M: Insecure Conversion of Amount

TypeSeverityLocation
Mathematical OperationsSafetyTransfer.sol:L22

Description:

The SafetyTransfer::decimalsCorrection function is utilized to assess how much funds should be transferred. As such, it is possible to specify a value that will truncate to 0 when "normalized" to the token's decimal accuracy, permitting deposits of zero funds to acquire a non-zero effective value in the protocol.

Impact:

It is currently possible to trick functions such as BorrowerOperations::_activePoolAddColl to perform a zero-value transfer yet credit a non-zero deposit value to the caller, significantly compromising the operational integrity of the protocol.

Example:

contracts/Dependencies/SafetyTransfer.sol
12function decimalsCorrection(address _token, uint256 _amount)
13 internal
14 view
15 returns (uint256)
16{
17 if (_token == address(0)) return _amount;
18 if (_amount == 0) return 0;
19
20 uint8 decimals = ERC20Decimals(_token).decimals();
21 if (decimals < 18) {
22 return _amount.div(10**(18 - decimals));
23 }
24
25 return _amount;
26}

Recommendation:

We advise the code to mandate that _amount modulo (%) the divisor (i.e. 10**(18 - decimals)) equals zero, preventing impossible deposit values from being specified.

Alleviation:

The SafetyTransfer::decimalsCorrection function will now validate that the amount being converted is fully divisible via a modulo (%) operator, ensuring that the code will never yield assets that are less than the expected amount.