search

code-423n4 / 2022-02-skale-findings

0 stars 0 forks source link

QA Report #56

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Schain owner can censor user by not distributing sFUEL

SKALE chains operate in a gas-free environment using a native gas token called sFUEL. sFUEL has no economic value and is allocated from the SKALE chain owner

It is possible for schain owner to censor user / freeze user fund by not distributing enough sFUEL

setMinTransactionGas lack minimum value

Since the base Tx gas cost is 21000, some global minimum can be hardcoded to avoid misconfiguration https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/CommunityPool.sol#196

_balanceIsSufficient use current tx.gasprice but the actual gasprice node will pay is unknown

https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/CommunityPool.sol#220

Upgrade Solidity Version

Recommended to upgrade to latest Solidity 0.8.12

Unresolved TODO

contracts/schain/CommunityLocker.sol:219:        // TODO: uncomment when oracle finished

Out-of-order encode-decode function

Some of the decode function is not adjacent to its encode counterpart, which make reading the code a bit annoying (e.g. encodeTransferErc20Message and decodeTransferErc20Message) https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/Messages.sol

depositERC20 lack input validation

DepositBoxERC20.depositERC20() did not check if amount!=0 https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/DepositBoxes/DepositBoxERC20.sol

DimaStebaev commented 2 years ago

Agreed.

GalloDaSballo commented 2 years ago

Schain owner can censor user by not distributing sFUEL

Given that this is effectively a design decision, I agree with Low Severity

setMinTransactionGas lack minimum value

Agree with lack of validation given rational values

_balanceIsSufficient use current tx.gasprice but the actual gasprice node will pay is unknown

Would have liked to see this finding more developed

Upgrade Solidity Version

In lack of explanation I am not going to count this finding

Out-of-order encode-decode function

That's like, your opinion man.

depositERC20 lack input validation

Valid