LI.FI contest - tchkvsky's results

Declared event in WithdrawFacet.sol doesn't match emitted event

Handle

tchkvsky

Vulnerability details

Declared event in #L12 of WithdrawFacet.sol doesn't match emitted event (called parameters) in #L37

Impact

This could lead to logging incorrect transactions

Proof of Concept

WithdrawFacet.sol#L12

event LogWithdraw(address indexed _assetAddress, address _from, uint256 amount);

and

WithdrawFacet.sol#L37

    emit LogWithdraw(sendTo, _assetAddress, _amount);

does not match

Tools Used

Manual review

Recommendation

Consider changing (#L12)

event LogWithdraw(address indexed _assetAddress, address _from, uint256 amount);

to

event LogWithdraw(address indexed _assetAddress, address _to, uint256 _amount);

AND (#L37)

    emit LogWithdraw(sendTo, _assetAddress, _amount);

to

    emit LogWithdraw(_assetAddress, sendTo, _amount);

Useful links:

• https://eips.ethereum.org/EIPS/eip-20#events

• https://docs.soliditylang.org/en/v0.8.13/contracts.html#events

Prefix increments are cheaper than postfix increments.

Handle

tchkvsky

Vulnerability details

These functions use not using prefix increments (++x)

Impact

Using prefix increment is more gas efficient

Proof of Concept

DexManagerFacet.sol

#L33

#L52

#L65

Swapper.sol

#L14

Tools Used

Manual review

Recommendation

Consider using prefix increments (++i) to save gas