Contract can over-pay or under-pay the gas refund payment if the gas payment ERC20 token is not 18 decimals.

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-biconomy/blob/53c8c3823175aeb26dee5529eeefa81240a406ba/scw-contracts/contracts/smart-contract-wallet/SmartAccount.sol#L264
https://github.com/code-423n4/2023-01-biconomy/blob/53c8c3823175aeb26dee5529eeefa81240a406ba/scw-contracts/contracts/smart-contract-wallet/SmartAccount.sol#L239

Vulnerability details

Impact

Gas refund amount can be inaccurate if the gas payment ERC20 token is not 18 decimals.

Proof of Concept

In the current implementation, the code handle the gas refund payment logic below:

if (refundInfo.gasPrice > 0) {
	//console.log("sent %s", startGas - gasleft());
	// extraGas = gasleft();
	payment = handlePayment(startGas - gasleft(), refundInfo.baseGas, refundInfo.gasPrice, refundInfo.tokenGasPriceFactor, refundInfo.gasToken, refundInfo.refundReceiver);
	emit WalletHandlePayment(txHash, payment);
}

which calls:

function handlePayment(
	uint256 gasUsed,
	uint256 baseGas,
	uint256 gasPrice,
	uint256 tokenGasPriceFactor,
	address gasToken,
	address payable refundReceiver
) private nonReentrant returns (uint256 payment) {
	// uint256 startGas = gasleft();
	// solhint-disable-next-line avoid-tx-origin
	address payable receiver = refundReceiver == address(0) ? payable(tx.origin) : refundReceiver;
	if (gasToken == address(0)) {
		// For ETH we will only adjust the gas price to not be higher than the actual used gas price
		payment = (gasUsed + baseGas) * (gasPrice < tx.gasprice ? gasPrice : tx.gasprice);
		(bool success,) = receiver.call{value: payment}("");
		require(success, "BSA011");
	} else {
		payment = (gasUsed + baseGas) * (gasPrice) / (tokenGasPriceFactor);
		require(transferToken(gasToken, receiver, payment), "BSA012");
	}
	// uint256 requiredGas = startGas - gasleft();
	//console.log("hp %s", requiredGas);
}

If the gasToken is address(0), the gas refund is paid in ETH. the value gasUsed, baseGas and tx.gasprice are all in 18 decimals because the ETH is in 18 decimals.

The gas price is assumed to be calculated in 18 decimals as well

However, if the gasToken is not address(0) and the gas payment is settled in ERC20 token, the token decimals can be less than 18 decimals or more than 18 decimals.

According to

https://github.com/d-xo/weird-erc20#low-decimals

Some tokens have low decimals (e.g. USDC has 6). Even more extreme, some tokens like Gemini USD only have 2 decimals.

and

https://github.com/d-xo/weird-erc20#high-decimals

Some tokens have more than 18 decimals (e.g. YAM-V2 has 24).

However, the gasUsed and baseGas and gasPrice is still in 18 decimalls.

If the ERC20 token is not in 18 decimals, the payment amount can be over-valued if the ERC20 token is less than 18 deciamls or under-valued if the ERC20 token is more than 18 decimals.

Let us use USDC (a token that has decimals 6) as an example:

consider the code below:

payment = (gasUsed + baseGas) * (gasPrice) / (tokenGasPriceFactor);

let us say, the gasUsed is 21000 WEI, base Gas is 20000 WEI, gasPrice is 1000000 WEI, tokenGasPriceFactor is 10.

payment = 21000 WEI * 20000 WEI * 1000000 WEI / 10 = 4100000000 WEI

If the payment is setlled in a 18 decimals token, for example, BNB, 4100000000 WEI is a fraction of 1000000000000000000

However, if the payment is settled in USDC, 4100000000 WEI is around 4100 USDC, if we use 4100000000 / (10 ** 6).

Clearly we over-refund the gas payment.

If the Smart Account hold enough ERC20 balance, for enough 4100 UDSC, the gas refund payment can sliently go through.

the math division by tokenGasPriceFactor can reduce the impact of this issue but there is no boundary check and precision check built in-place for this parameter tokenGasPriceFactor

The code cannot the token that is more than 18 decimals as well, because the math division by tokenGasPriceFactor only scale the payment amount down but if the token is more than 18 decimals, the payment should be scaled up

Tools Used

Manual Review

Recommended Mitigation Steps

We recommend the protocol only settle the payment in ETH to avoid such issue or whitelist and validate the payment precision beforing using the token to settle the gas refund.

[c4-judge]

gzeon-c4 marked the issue as duplicate of #492

[c4-sponsor]

livingrockrises marked the issue as sponsor confirmed

[c4-judge]

gzeon-c4 changed the severity to 3 (High Risk)

[c4-judge]

gzeon-c4 marked the issue as satisfactory