MintingFactoryV2, BaseUpgradableMarketplace & KODAV3UpgradableGatedMarketplace – CoinFabrik Weblog

Home » MintingFactoryV2, BaseUpgradableMarketplace & KODAV3UpgradableGatedMarketplace – CoinFabrik Weblog

Introduction

CoinFabrik was requested to audit the contracts for the KnownOrigin mission. First we
will present a abstract of our discoveries after which we’ll present the main points of our
findings.

Scope

The contracts audited are from the
https://github.com/knownorigin/known-origin-contracts-v3.git git repository. The
audit relies on the commit d592c5f4fa4e0b6fc65a1fce43e302706aedf607.

The audited information are:

/contracts/market/KODAV3UpgradableGatedMarketplace.sol:
Comprises the contract used to deploy gated marketplaces.
/contracts/market/BaseUpgradableMarketplace.sol: Comprises
the BaseUpgradableMarketPlace contract. This contract can be utilized to
make upgradable marketplaces, together with a number of utilities that simplify its
creation.
/contracts/minter/MintingFactoryV2.sol: Comprises the
MintingFactoryV2 contract. This contract is an upgradeable glue contract
that binds a number of contracts to facilitate the minting course of. It’s supposed to
exchange the MintingFactory contract whereas permitting for gated marketplaces.
/contracts/entry/IKOAccessControlsLookup.sol: It incorporates the
interface definition for the contract chargeable for dealing with authorization of
completely different roles within the system.
/contracts/core/IKODAV3.sol: It incorporates the interface definition for the
KODAV3 token.
/contracts/market/IKODAV3Marketplace.sol: Comprises interface
definitions for various sorts of market contracts.
/contracts/core/IKODAV3Minter.sol: Comprises the interface definition for
the KODA Minter contract (model 3).
/contracts/collab/ICollabRoyaltiesRegistry.sol: Comprises the
interface definition for the royalties registry.

The scope of the audit is proscribed to these information. No different information on this repository had been
audited. Its dependencies are assumed to work in accordance with their documentation.
Additionally, no checks had been reviewed for this audit.
Fixes had been checked on commit 3bcd94f66e5d0f6b38881fd52971c13dd08b6974.

After that the event staff fastened a bug concerning royalties distribution. We
checked that repair on commit 9cd490474667bf021c7b0c1e8b3c697fc3072f3a and no
new safety points had been discovered within the audited information.

Analyses

With out being restricted to them, the audit course of included the next analyses:

● Arithmetic errors
● Outdated model of Solidity compiler
● Race circumstances
● Reentrancy assaults
● Misuse of block timestamps
● Denial of service assaults
● Extreme fuel utilization
● Lacking or misused operate qualifiers
● Needlessly advanced code and contract interactions
● Poor or nonexistent error dealing with
● Inadequate validation of the enter parameters
● Incorrect dealing with of cryptographic signatures
● Centralization and upgradeability

Abstract of Findings

We discovered no important or medium points. A number of minor points had been discovered. Additionally,
a number of enhancements had been proposed.
All safety points had been both resolved, mitigated or acknowledged by the
improvement staff. Some enhancements had been carried out by the event
staff.

Safety Points

Privileged Roles

These are the privileged roles that we recognized on every of the audited contracts.

MintingFactoryV2

Admin

An handle with the admin position can:

● Improve the contract.
● Set and unset the frequency override for an handle.
● Set the minting interval.
● Set the royalties registry.
● Set the utmost mints in a interval.

The accessControls contract controls if an handle has the admin position through the
hasAdminRole() operate.

Verified Artist

An handle with the verified artist position can mint batches for itself utilizing the
following capabilities:

● mintBatchEdition()
● mintBatchEditionGatedOnly()
● mintBatchEditionGatedAndPublic()
● mintBatchEditionOnly()

Minting is ruled by the restrictions set by the admin.
The accessControls contract controls if an handle has the verified artist position through
the isVerifiedArtist() operate. The required cryptographic proof is forwarded
to this operate.

Verified Artist Proxy

An handle with the verified artist proxy position can mint batches on behalf of a
creator it represents utilizing the next capabilities:

● mintBatchEditionAsProxy()
● mintBatchEditionGatedOnlyAsProxy()
● mintBatchEditionGatedAndPublicAsProxy()
● mintBatchEditionOnlyAsProxy()

Minting is ruled by the restrictions set by the admin.
The accessControls contract controls if an handle is a verified artist proxy for a
creator through the isVerifiedArtistProxy() operate.

BaseUpgradableMarketplace

Admin

An handle with the admin position can:

● Improve the contract.
● Change the accessControls contract used within the contract through the
updateAccessControls() operate. So as to take action, it must have the
admin position in each the previous and new accessControls contracts.
● Switch away ERC20 tokens owned by the contract through the recoverERC20()
operate.
● Switch ether away owned by the contract the recoverStuckETH()
operate.
● Replace the platform commision through the
updatePlatformPrimaryComission() operate. This worth isn’t utilized in
this contract however could also be utilized in a derived contract.
● Replace the modulo worth through the updateModulo() operate. This worth is simply
used within the personal and non-invoked _handleSaleFunds() operate and should
even be utilized in a derived contract.
● Replace the minBidAmount through the updateMinBidAmount() operate. This
worth isn’t used on this contract however could also be utilized in a derived contract.
● Replace the bidLockupPeriod through the updateBidLockupPeriod() operate.
This worth is simply used within the personal and non-invoked _getLockupTime()
operate and may be utilized in a derived contract.

● Replace the platformAccount worth through the updatePlatformAccount()
operate. This worth is simply used within the personal and non-invoked
_handleSaleFunds() operate and may be utilized in a derived contract.
● Pause and resume the contract through the pause() and unpause() capabilities.

The accessControls contract controls if an handle has the admin position through the
hasAdminRole() operate.
Different capabilities in derived contracts could also be allowed to an admin solely through the
onlyAdmin() modifier and/or to the admin and different roles through the
onlyCreatorContractOrAdmin() modifier.

Contract and Creator

Whereas there aren’t any capabilities marked with the onlyCreatorContractOrAdmin()
modifier on this contract, this modifier can be utilized to limit the execution of
capabilities to both an admin, a contract or the issuer of the version handed as a
parameter. Checking that an handle has a contract or admin position is made through the
accessControls.hasContractOrAdminRole() operate. To examine if an handle
corresponds to an editionId the koda.getCreatorOfEdition() operate is used.

KODAV3UpgradableGatedMarketplace

This contract inherits all of the roles and capabilities outlined within the
BaseUpgradableMarketplace contract. No new roles are added, however the authentic
roles get extra capabilities.

Admin

In addition to the capabilities outlined in BaseUpgradableMarketplace contract, an
handle with the admin position can:

● Change the fundsReceiver of a sale through the updateFundsReceiver()
operate.
● Change the maxEditionId of a sale through the updateMaxEditionId()
operate.
● Replace the creator of a sale through the updateCreator() operate.
● Override the fee for a sale through the
setKoCommissionOverrideForSale() operate.

Admin and Contract

An handle with the admin and/or contract roles can:

● Create gross sales for any version through the createSale() and
createSaleWithPhases() capabilities.
● Create and take away phases of gross sales for any version through the createPhase(),
createPhases() and removePhase() capabilities.
● Pause or resume any sale through the toggleSalePause() operate.

Creator

An handle with the admin and/or contract roles can:

● Create gross sales for any version whose editionId corresponds to them through the
createSale() and createSaleWithPhases() capabilities.
● Create and take away phases of gross sales for any version whose editionId
corresponds to them through the createPhase(), createPhases() and
removePhase() capabilities.
● Pause or resume any sale whose editionId corresponds to them through the
toggleSalePause() operate.

Safety Points Discovered

Severity Classification

Safety dangers are categorised as follows:

Crucial: These are points that we handle to use. They compromise the
system critically. They should be fastened instantly.
Medium: These are probably exploitable points. Though we didn’t
handle to use them or their affect isn’t clear, they may signify a
safety threat within the close to future. We propose fixing them as quickly as doable.
Minor: These points signify issues which are comparatively small or troublesome
to benefit from however might be exploited together with different points.
These sorts of points don’t block deployments in manufacturing environments.
They need to be taken into consideration and be fastened when doable.

Points Standing

A difficulty detected by this audit can have 4 distinct statuses:

Unresolved: The difficulty has not been resolved.
Acknowledged: The difficulty stays within the code however is a results of an intentional
determination.

Resolved: Adjusted program implementation to eradicate the chance.
Partially resolved: Adjusted program implementation to eradicate a part of the
threat. The opposite half stays within the code however is a results of an intentional
determination.
Mitigated: Carried out actions to attenuate the affect or chance of the
threat.

Crucial Severity Points

No points discovered.

Medium Severity Points

No points discovered.

Minor Severity Points

MI-01 Doable Reentrancy Points

Location:

● contracts/minter/MintingFactoryV2.sol

Whereas accessControls, koda, market, gatedMarketplace and
royaltiesRegistry contracts are thought-about reliable, a artful attacker could
ultimately discover a means to make use of these to set off a reentrancy assault within the
MintingFactoryV2 contract.

Suggestion

Mark all of the MintingFactoryV2.mint*() capabilities as non-reentrant utilizing the
nonReentrant modifier outlined within the OpenZeppelin library. Reentrancy points are
not simple to identify and the checks-effects-interaction sample can’t be trivially
utilized on this contract.

Standing

Acknowledged.

MI-02 Outdated Solidity Model

The solidity model declared within the audited contracts is 0.8.4. The most recent model
launched on the time of this audit is 0.8.13.

Suggestion

Check all contracts with the newest model and alter the pragma solidity
statements.

Standing

Acknowledged.

MI-03 Unchecked Switch

Location:

● contracts/market/BaseUpgradableMarketplace.sol:92

When the admin recovers ERC20 tokens within the BaseUpgradableMarketplace
contract, the contract doesn’t management if the switch to the brand new handle is made or
not.
This challenge is minor as a result of:

● Solely the admin can get well ERC20 tokens.
● If the examine fails, the one distinction is {that a} AdminRecoverERC20 occasion is
emitted when it shouldn’t be.

Suggestion

Use OpenZeppelin’s safeTransfer() operate to switch ERC20s.

Standing

Resolved. Checked on commit 3bcd94f66e5d0f6b38881fd52971c13dd08b6974.

MI-04 Doable DOS on KODAV3UpgradableGatedMarketplace.mint()

Location:

● contracts/market/BaseUpgradableMarketplace.sol:149,152

When KODAV3UpgradableGatedMarketplace.mint() is known as, ultimately
BaseUpgradableMarketplace._handleSalesFunds() is known as a number of occasions. In
this operate, 2 ether transfers are made, one to the platformAddress and the
different to the fundsReceiver of the sale.
If any of these addresses rejects the switch, then the minting is aborted.
The severity of this challenge was lowered as a result of neither the platform nor the fund
receiver has clear financial incentives to do that and the DOS is transient.

Suggestion

Use the withdrawal sample to switch funds to these addresses. This will likely additionally
end in fuel financial savings for a number of mints, as a single switch could be required for
every handle.

Standing

Mitigated. The event staff knowledgeable us that they don’t need to introduce the
pull sample at this level resulting from UX complexity. mitigation might be achieved by an
admin operating the
KODAV3UpgradableGatedMarketplace.updateFundsReceiver() operate. The
improvement staff additionally informs that the funds handler is derived from the collab
registry, to allow them to additionally disable minting entry to the malicious artists if discovered and
additionally replace the funds handler on this state of affairs.

Enhancements

These things don’t signify a safety threat. They’re greatest practices that we
counsel implementing.

Desk

Particulars

EN-01 Time Lock Mechanism to Switch Funds and Improve

Location:
● contracts/market/BaseUpgradableMarketplace.sol:87-100

If an attacker takes management of an handle with the admin position, it could actually steal all of the
funds belonging to a BaseUpgradableMarketplace derived contract, together with

https://docs.soliditylang.org/en/v0.8.13/common-patterns.html#withdrawal-from-contracts

KODAV3UpgradableGatedMarketplace. This operation could also be made immediately,
giving the true admins no recourse to cease the operation.

Suggestion

Make the operations to get well ERC20s and ether, and in addition the improve of the
contract, time locked. This may give the chance to stop this assault from
taking place.

Standing

Not carried out. The event staff acknowledges this advice however
won’t implement it.

EN-02 Lacking Non-Zero Checks

● contracts/market/BaseUpgradableMarketplace.sol:78,96,131

The BaseUpgradableMarketplace contract is lacking the checks for zero
addresses, that are carried out in its little one contract
KODAV3UpgradableGatedMarketplace.

This will likely result in utilizing an uninitialized handle as goal for a switch, shedding these
funds.

Suggestion

Add the non-zero checks to the next operate parameters:

_platformAccount within the BaseUpgradableMarketplace.initialize()
operate.
● _recipient within the BaseUpgradableMarketplace.recoverStuckETH()
operate.
● _newPlatformAccount within the
BaseUpgradableMarketplace.updatePlatformAccount() operate.

Standing

Carried out. Checked on commit 3bcd94f66e5d0f6b38881fd52971c13dd08b6974.

EN-03 Commissions Calculations

Location:

● contracts/market/BaseUpgradableMarketplace.sol:29-32,
58-67, 113-117, 146-154
● contracts/market/KODAV3UpgradableGatedMarketplace.sol:70,
102-105, 306-311, 396-401

There are a number of issues that may be improved within the commissions calculations:

  1. Decouple modulo for every commision. Now if the admin modifications the modulo
    then all of the commissions could be altered. It could be higher for every
    fee to have its personal modulo.
  2. The worth for koCommissionOverrideForSale[_saleId].koCommission
    (set in KODAV3UpgradableGatedMarketplace.sol) and
    platformPrimaryCommission (set in BaseUpgradableMarketplace.sol)
    ought to be lower than its corresponding modulo.
  3. Provided that the present provide of wei is about 1.2e26 , the multiplication in
    2 line 147 of BaseUpgradableMarketplace.sol wouldn’t overflow if written
    as msg.worth * _platformCommission / modulo, provided that _platform fee is lower than 10 50. Checking that’s lower than 10 40 would be certain that it might not overflow for millennia. And by multiplying earlier than dividing the calculation of the fee could be extra exact. If different blockchains are added (completely different from ethereum), these numbers ought to be revised
    accordingly to keep away from overflows.

Standing

Partially carried out. Hereunder there may be the cut up of this advice
implementation:

  1. Not carried out.
  2. Not carried out.
  3. Carried out. Checked on commit
    3bcd94f66e5d0f6b38881fd52971c13dd08b6974.

Different Concerns

The issues said on this part usually are not proper or unsuitable. We don’t counsel
any motion to repair them. However we think about that they could be of curiosity for different
stakeholders of the mission, together with customers of the audited contracts, house owners or
mission traders.

Trusted Contracts

When analyzing the MintingFactoryV2 contract we assumed that the
accessControls, koda, market, gatedMarketplace and royaltiesRegistry contracts are reliable. These contracts are set within the initialization section, which is invoked in the course of the deployment of the contract.
When analyzing the KODAV3UpgradableGatedMarketplace contract and its guardian,
BaseUpgradableMarketplace, we assumed that the accessControls and koda
contracts are reliable. These contracts are set within the initialization section, which
is invoked in the course of the deployment of the contract. The accessControls contract can
even be modified by an admin, utilizing the
BaseUpgradableMarketplace.updateAccessControls() operate.

Centralization and Upgrades

The contracts MintingFactoryV2, KODAV3UpgradableGatedMarketplace and
BaseUpgradableMarketplace are fairly centralized. Particularly, an handle with
the admin position could set off an improve, so having the ability to run any code. An
administrator may switch any ERC20 or ether belonging to a
BaseUpgradableMarketplace derived contract, together with
KODAV3UpgradableGatedMarketplace, to any arbitrary handle.
Within the KODAV3UpgradableGatedMarketplace contract, addresses with the contract
position even have important capabilities to intrude with any sale.

Changelog

● 2022-04-12 – Preliminary report primarily based on commit
d592c5f4fa4e0b6fc65a1fce43e302706aedf607.
● 2022-04-18 – Examine fixes on commit
3bcd94f66e5d0f6b38881fd52971c13dd08b6974.
● 2022-04-28 – Examine bug repair on commit
9cd490474667bf021c7b0c1e8b3c697fc3072f3a.

Disclaimer: This audit report isn’t a safety guarantee, funding recommendation, or an
approval of the KnownOrigin mission since CoinFabrik has not reviewed its
platform. Furthermore, it doesn’t present a sensible contract code faultlessness
assure.

Leave a Reply

Your email address will not be published.