GigaSwap Position Trading

Smart Contract Audit Report

Audit Summary

GigaSwap Position Trading Audit Report GigaSwap is building a new platform where users can initiate ERC-20, ERC-721, and ETH position trades.

For this audit, we reviewed the Sale, PositionsController, Erc20AssetFactory, Erc20Asset, Erc721ItemAssetFactory, Erc721ItemAsset, ETHAssetFactory, and ETHAsset contracts provided to us by the project team.

Audit Findings

High findings were identified and the team must resolve these issues.
Date: August 8th, 2022.
Finding #1 - Sale - High
Description: Any user can call the afterAssetTransfer() function and send tokens or ETH from any owners asset contract to any address at no cost.
function afterAssetTransfer(uint256 positionId, address asset, address from, address to, uint256 amount ) external override {
    _afterAssetTransfer(positionId, asset, from, to, amount);
}
...
function _afterAssetTransfer(uint256 positionId, address asset, address from, address to, uint256 amount) internal virtual override {
...
    ownerAsset.withdraw(from, buyCount);
Risk/Impact: The tokens or ETH in the owners asset contracts can be withdrawn by any user at no cost.
Recommendation: The team should modify this function to only allow an exchange of tokens or ETH to be made, or to fit their intended functionality.
Resolution: The team has not yet addressed this issue.

Finding #2 - Sale - High
Description: The _beforeAssetTransfer() function checks the below conditions but does not execute any logic based on the result.
function _beforeAssetTransfer(uint256 positionId, address asset, address from, address to, uint256 amount ) internal override {
    address ownerAsset = positionsController .ownerAsset(positionId).contractAddr;
    if (asset != ownerAsset || from != ownerAsset)
    return;
Risk/Impact: There may be unintended consequences from checking these conditions and not acting on them in any case.
Recommendation: The team should modify the _beforeAssetTransfer() function to fit their intended functionality.
Resolution: The team has not yet addressed this issue.

Finding #3 - Erc20Asset & Erc721ItemAsset & EthAsset - High
Description: The transferToAsset() function and receive() function transfers tokens/ETH from the "spender" address/msg.sender to the contract. If an algorithm is not set for the contract, the spender's tokens/ETH will not be exchanged for any asset.
function transferToAsset(address spender, uint256 amount) external {
...
IERC20(contractAddress).transferFrom(spender, address(this), amount);
if (alg != address(0))
IPositionAlgorithm(alg).afterAssetTransfer(positionId, address(this), spender, address(this), amount);
Risk/Impact: Users can mistakenly transfer tokens/ETH to the contract in exchange for nothing and will not be able to withdraw their assets.
Recommendation: The team should modify the receive() and transferToAsset() functions to not accept payment if an exchange will not be made.
Resolution: The team has not yet addressed this issue.

Finding #4 - Sale - High
Description: The buyCount calculation in the _afterAssetTransfer() function does not factor in decimals.
function _afterAssetTransfer(uint256 positionId, address asset, address from, address to, uint256 amount) internal virtual override {
...
uint256 buyCount = amount / price;
require(buyCount <= ownerAsset.count(),"not enough owner asset to buy");

ownerAsset.withdraw(from, buyCount);
Risk/Impact: If tokens with different decimal values are being swapped, the buyCount value that is sent to the user will be inaccurate.
Recommendation: The amount value should be multiplied by the decimals value of the owner asset before dividing by the price in the buyCount calculation when the owner asset is an ERC-20 token.
Resolution: The team has not yet addressed this issue.

Finding #5 - Erc20Asset & Erc721ItemAsset - Medium
Description: The transferToAsset() function allows any user to input a "spender" address that will transfer tokens to the contract.
function transferToAsset(address spender, uint256 amount) external {
...
IERC20(contractAddress).transferFrom(spender, address(this), amount);
...

function transferToAsset(address spender) external {
...
IERC721(contractAddress).transferFrom(spender, address(this), tokenId);
Risk/Impact: Any user can initiate a transfer on behalf of another user if any excess allowance has been granted to the contract by the "spender".
Recommendation: The "spender" address should be removed as a parameter and msg.sender should instead be used throughout the transferToAsset() functions.
Resolution: The team has not yet addressed this issue.

Finding #6 - Erc721ItemAsset - Medium
Description: The transferToAsset() function checks the condition: if (alg != address(0)) before transferring tokens but does not check this condition before calling the afterAssetTransfer() function in the algorithm contract.
function transferToAsset(address spender) external {
...
    if (alg != address(0))
        IERC721(contractAddress).transferFrom(
            spender,
            address(this),
            tokenId
        );
    IPositionAlgorithm(alg).afterAssetTransfer(
        positionId,
        address(this),
        spender,
        address(this),
        1
    );
}
Risk/Impact: The transaction will revert on all transfers when an algorithm is not set as the function would be attempting to call the afterAssetTransfer() function in the 0x00 address.
Recommendation: The team should modify the transferToAsset() function to fit their intended functionality.
Resolution: The team has not yet addressed this issue.

Finding #7 - Sale - Low
Description: Any excess tokens/ETH supplied to the contract during a trade is not returned to the user.
function _afterAssetTransfer(uint256 positionId, address asset, address from, address to, uint256 amount) internal virtual override {
...
require(amount >= price, "not enough amount to buy - see price");
Risk/Impact: Users will lose any excess funds sent as payment.
Recommendation: The contract should ensure that only the exact amount of tokens needed to satisfy a trade is supplied if the payment is made with an ERC-20 token, and any excess ETH should be returned to the user if the payment is with ETH.
Resolution: The team has not yet addressed this issue.

Finding #8 - Sale - Informational
Description: The asset parameter passed into the _afterAssetTransfer() function is never utilized.
Recommendation: The team should modify this function to fit their intended functionality.

Contracts Overview

PositionsController Contract:

Any user can create a new position at any time. The position ID assigned to a new position is 1 more than the previously created position ID.

This contract can be used to view the owner assigned to a position ID, as well as the owners asset, output asset, and algorithm associated with it.

Sale Contract:

The owner of a position can set an algorithm for their asset at any time.

The owner of a position can lock their unlocked owner asset for a specified amount of time.

The owner of a position can set the price of their position when unlocked.

The owner of a position can withdraw an unlocked owner asset and an unlocked output asset which will send the desired number of tokens/ETH from the asset contract to a specified recipient address.

Erc20AssetFactory and Erc20Asset Contracts:

The owner of a position can add an owner asset and an output asset to their existing position by specifying a position ID and a token address.

A new Erc20Asset contract is created when an owner asset and an output asset are added.

The owner of a position can update the owner asset and output asset of their position at any time.

Any user can initiate a transfer in the output asset contract by specifying a spender address and a number of tokens.

The tokens will be transferred from the spender's address to the contract for the specified amount. The spender must grant the contract an approval in order for this transaction to successfully occur.

A buy count in the Sale contract is calculated based on the proportion of the number of tokens entered by the user to the price assigned to the position by the position owner.

In return, the asset from the owner asset contract is transferred to the user for the buy count value.

The owner asset contract must have a sufficient token/ETH balance in order for this transaction to successfully occur.

Erc721ItemAssetFactory & Erc721ItemAsset Contracts

The owner of a position can add an owner asset and an output asset to their existing position by specifying a position ID, an NFT address, and a Token ID.

A new Erc721ItemAsset contract is created when an owner asset and an output asset are added.

Any user can initiate a transfer in the output asset contract by specifying a spender address.

The NFT will be transferred from the spender's address to the contract. The spender must grant the contract an approval in order for this transaction to successfully occur.

A buy count in the Sale contract is calculated based on the proportion of the number of tokens entered by the user to the price assigned to the position by the position owner.

In return, the asset from the owner asset contract is transferred to the user for the buy count value.

The owner asset contract must have a sufficient token/ETH balance in order for this transaction to successfully occur.

ETHAssetFactory and ETHAsset Contracts:

The owner of a position can add an owner asset and an output asset to their existing position by specifying a position ID and a token address.

A new ETHAsset contract is created when an owner asset and an output asset are added.

The owner of a position can update the owner asset and output asset of a position at any time.

Any user can initiate a transfer in the output asset contract by transferring ETH to the contract.

A buy count in the Sale contract is calculated based on the proportion of the ETH entered by the user to the price assigned to the position by the position owner.

In return, the asset from the owner asset contract is transferred to the user for the buy count value.

The owner asset contract must have a sufficient token/ETH balance in order for this transaction to successfully occur.