G-N354X1RGVT
BlockchainToday

Recognized Origin Digital Asset Audit

Introduction

CoinFabrik was requested to audit the contracts for the Recognized Origin Digital Asset v3 undertaking. First we’ll present a abstract of our discoveries after which we’ll present the main points of our findings.

Abstract

The contracts audited are from the Recognized Origin Subsequent Era Mission repository at this hyperlink
(https://github.com/knownorigin/known-origin-contracts-next-gen). The audit began based mostly on the commit bcbd6b7bfe7e69ef44cb7d1ed524ca810a86d2dc, and was up to date to mirror modifications based mostly on this second commit: bede703e34f5ca4878cc5dcebc7a2f9fa649b189.
The undertaking consists of a number of contracts (listed beneath) which outline a token and a market to deal with the gross sales of those tokens. 4 kinds of itemizing sorts are supported: Purchase now, Stepped auctions, Reserve auctions, and Gives.

Contracts

The audited contracts are:

● core/BaseKoda.sol: Fundamental summary contract with primary token performance
● core/KnownOiginDigitalAssetV3.sol: Primary token contract
● core/Konstants.sol
● core/IERC721Ownable.sol
● core/IERC2309.sol
● core/IERC2981.sol
● core/composable/TopDownERC20Composable.sol: summary contract defining
token.
● market/BaseMarketplace.sol: summary contract with primary market
performance
● market/KODAV3PrimaryMarketplace.sol: Contract dealing with major
gross sales
● market/KODAV3SecondaryMarketplace.sol: Contract dealing with
secondary gross sales
● market/BuyNowMarketplace.sol: Contract handline on-the-moment
gross sales.
● market/ReserveAuctionMarketplace.sol: Contract dealing with Auctions.

Analyses

The next analyses had been carried out:

● IMisuse of the totally different name strategies
● Integer overflow errors
● Division by zero errors
● Outdated model of Solidity compiler
● Entrance operating assaults
● Reentrancy assaults
● Misuse of block timestamps
● Softlock denial of service assaults
● Features with extreme fuel price
● Lacking or misused perform qualifiers
● Needlessly complicated code and contract interactions
● Poor or nonexistent error dealing with
● Failure to make use of a withdrawal sample
● Inadequate validation of the enter parameters
● Incorrect dealing with of cryptographic signatures

Severity Classification

Safety dangers are labeled as follows:

Essential: These are points that we handle to take advantage of. They compromise the
system severely. They have to be mounted instantly.
Medium: These are doubtlessly exploitable points. Despite the fact that we didn’t
handle to take advantage of them or their affect shouldn’t be clear, they could characterize a
safety danger within the close to future. We advise fixing them as quickly as potential.
Minor: These points characterize issues which can be comparatively small or difficultto reap the benefits of however will 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 mounted when potential.
Enhancement: These sorts of findings don’t characterize a safety danger. They’re greatest practices that we advise to implement.

This classification is summarized within the following desk:

Findings and Fixes

The desk beneath summarizes findings. Particulars comply with within the subsequent part.

Points Discovered by Severity

Essential severity

No points discovered

Medium severity

ME-01 Inadequate Enter Validation

The perform isEditionSoldOut within the KnownOriginDigitalAssetV3.sol contract doesn’t examine whether or not the sought editionId exists, therefore when queried with an inexistent tokenId the perform returns true.
This might lead customers to assume that an version exists when it doesn’t and will mislead actions like airdrops or reward mechanisms if the general public perform editionExists() shouldn’t be used beforehand.

Advice

Add a require assertion like in comparable features. For instance,
require(_editionExists(_editionId), “Version doesn’t exist”);

Standing

The advice was utilized and the problem not exists.

ME-02 Bid Deniability in placeTokenBid() Perform

In placeTokenBid(uint256 _tokenId) and rejectTokenBid(uint256
_tokenId)
features (repeated within the KodaV3PrimaryMarketplace.sol and KodaV3SecondaryMarketplace.sol) there are calls to _refundBidder() in these instances the place an earlier provide was made. Since _refundBidder() requires the refund to achieve success, a bidder might block others from putting larger bids by merely refusing the switch.
The malicious bidder would then win the bid, manipulating the results of the public sale.

Advice

Decouple bidding from refund (the success of the refund shouldn’t be required for a brand new bid to be positioned).

Refunds are positioned in a refund queue however the bid is instantly changed. For instance, add a construction to retailer missed refunds which acts as a queue and a periodic process which handles these refunds.

Standing

A profitable refund is not required for somebody to outbid an present bid. The difficulty not exists. Documentation ought to mirror this.

Minor Severity

MI-01 Misplaced Gives Due To Defective Design

A disadvantage on the bidding logic in KODAV3PrimaryMarketplace.sol might end in dropping legitimate provides when a sale shouldn’t be made earlier than the bidLockupPeriod. An instance follows: a primary purchaser (buyer_A) locations a suggestion. Because of this, the editionOffers mapping is up to date as follows:

editionOffers[_editionId] = Supply(msg.worth, (1)
_msgSender(),
_getLockupTime());

Subsequent, a second purchaser (buyer_B) locations a second and higher provide, and because of this buyer_A is refunded and this second provide replaces provide (1) above, thus updating editionOffers. If the lockup interval has elapsed (timestamp >= lockupUntil), then buyer_B could cancel the provide utilizing withdrawEditionBid() later and no bid stays.

Advice

Enable bidders to optionally preserve their bid, even when it’s not the perfect bid, by storing dropping bids till the public sale is completed.

Standing

Growth crew claims that is the anticipated conduct, explaining that the lockout interval could also be chosen to stop this, and off-chain occasions (corresponding to emails) notify homeowners when new bids are acquired. These actions do mitigate the problem thus minimizing the chance.

MI-02 Bidder with Greatest Bid Might Be Ignored

When a token creator calls convertOffersToBuyItNow() and there already is a suggestion, the bidder is refunded. This occurs even within the case when

provide.provide >= _listingPrice

wherein case, the bidder may very well be outpaced by one other purchaser. Furthermore, this new purchaser may very well be paying an quantity smaller than provide.provide! Therefore, each the preliminary bidder and token creator would lose.

Advice

Enable bidders to optionally have their bids mechanically transformed into buys if the creator converts the provide from sort ‘itemizing’ to ‘purchase now’.

Standing

Growth crew claims that is the anticipated conduct, explaining that the proprietor can change from “provide” to “purchase now” at his discretion. Documentation ought to thus mirror this, so homeowners could make knowledgeable selections.

MI-03 Entrance-running Assault on acceptTokenBid()

A malicious bidder might pay a smaller-than-expected value utilizing a front-running assault on the bid-acceptance name. Assume the next state of affairs:

  1. a bidder made a successful provide (e.g., by the perform placeTokenBid()),
  2. _offerPrice is strictly smaller than provide.provide,
  3. the lockupUntil time has elapsed, and
  4. the proprietor calls acceptTokenBid().

Then, the bidder might successively withdraw his preliminary provide and make one for offerPrice therefore saving provide.provide - _offerPrice.

Advice

Doc the affect of this entrance operating benefit and warn customers about this chance.

Standing

Growth crew claims that is the anticipated conduct, explaining that an knowledgeable proprietor can choose the worth _offerPrice at his discretion; therefore selecting this to be equal to provide.provide does eradicate the issue. Documentation ought to thus mirror this, so homeowners could make knowledgeable selections.

MI-04 Perform Enabling Arbitrary Callers Results in Public sale Deniability

An arbitrary contract could name listForReserveAuction on a token if that is an version of measurement one. Furthermore, if the token is already listed in a reserve public sale, every time somebody needs to name placeBidOnReserveAuction(), a malicious consumer might front-run him calling listForReserveAuction setting a reserveAuction.startDate sooner or later in order that the bid is rejected.

Advice

This subject wouldn’t exist if solely token creators are allowed to name this perform.

Standing

The advice was utilized and the problem was eradicated.

MI-05 Repeated Buildings Might Result in Knowledge Mismatch

The parameters editionDetails[editionId].uri could also be additionally obtained by the tokenUriResolver interface (tokenUriResolver.editionURI(editionId)). In truth, the perform editionURI will use the latter if tokenUriResolverActive() returns True or else the previous.

Standing

Growth Group explains that the info mismatch is predicted because the token resolver will turn into the defacto returned information payload as soon as referred to as and set, tremendous seeding any authentic uri set on creation. The enterprise rule would repair the problem.

Enhancements

EN-01 Potential Discount in Variable Dimension

The variable editionSize is created as an uint96, nevertheless it holds integers which can be bounded by MAX_EDITION_SIZE which is initialized as 1000. We will have editionSize to be uint16 (permitting MAX_EDITION_SIZE to develop as much as 65k).

Standing

Not utterly mounted.

EN-02 Untight Packing Resulting in Extra Gasoline Spent

The perform getEditionDetails(uint256 _tokenId) within the
KnownOriginDigitalAssetV3.sol contract returns the next values

(handle _originalCreator, handle _owner, uint256 _editionId, uint256 _size, string reminiscence _uri)

of respective sizes 160, 160, 256, (256), 59. These are usually not tightly packed into 32 byte slots and therefore might result in inefficient fuel spends.

Advice

Modifying the output in order that _size is of the smallest potential uint and putting uint256 return values at the beginning, reduces fuel.

Standing

Fastened.

EN-03 Variables with the identical title could result in issues

In each contracts BaseKoda.sol and KodaV3SecondaryMarketplace.sol a
variable named secondarySaleRoyalty is outlined, with the identical goal. In each contracts the worth is initialized as

uint256 public secondarySaleRoyalty = 12_50000

and will be modified solely by an admin. Nevertheless, the admin might name solely one of many replace features and the values would fall out of sync.

Standing

Growth crew understands the dangers and answered that there are enterprise processes in place to align modifications in each variables.

EN-04 Spurious Knowledge in editionDetails

The struct editionDetails incorporates three gadgets: the creator, editionSize and an URI of a token. Nevertheless, the URI may additionally be saved in one other map which will be accessed by the proxy tokenUriResolver. So, ultimately, when the proxy is used for URIs, the editionDetails could have outdated info within the URI entry however needs to be up to date within the different two entries. This will result in synchronization issues

Standing

Growth Group explains that the token resolver will turn into the defacto
returned information payload as soon as referred to as and set, tremendous seeding any authentic uri set on creation. This fixes the problem.

EN-05 Division-by-Zero Not Prevented

The features updateModulo and updateBasisPointsModulo permit modulus to be set to zero. A ‘require‘ assertion ought to forestall this.

Standing

The difficulty has been mounted.

Related Observations

Inaccurate Examine With Negligible Impression

The implementation of _safeTransferFrom makes use of the code

uint256 receiverCodeSize;
meeting {
receiverCodeSize := extcodesize(_to)
}
if (receiverCodeSize > 0) {…}

which can be referred to as by a maliciously-crafted contract that seems to have code of measurement 0, and due to this fact the examine fails. This examine needs to be used with care even when the safety affect of the to_ being a contract is negligible.
A recognized outdated instance (a zero-length-code Attacker contract dishonest a Sufferer contract) follows:

Conclusion

The Recognized Origin undertaking introduces new buying and selling mechanisms on its market which introduce new assault vectors that haven’t been analyzed for the recognized mechanisms utilized in different revealed marketplaces. Particularly wherein refers to front-running assaults and proper cryptographic validations.
ME-01 subject is expounded to the issues brought on by conduct customers anticipate from a selected perform based mostly on its title. ME-02 subject describes a state of affairs wherein bids might get caught. Whereas there’s a perform that the proprietor might name to get better from these conditions, the system shouldn’t depend on this contingency mechanism.
The minor points we discovered are associated to an absence of documentation which will lead customers to misuse the features as supposed or not pay attention to the potential front-running benefits that might happen.
The enhancements we suggest are associated to decreasing fuel consumption,
stopping a division-by-zero state of affairs and mitigating the chance of getting totally different values for the variables which can be declared with the identical title in two totally different contracts. We additionally posed a related statement concerning the usage of an inaccurate technique to confirm whether or not a given handle is a great contract or not.
All medium-severity points have been mounted and minor-severity points have both been mounted, needs to be mounted by enterprise guidelines which can be to be utilized on-chain, or have been accepted as they’re a part of the anticipated behaviour. In that case, documentation and enterprise processes scale back the chance to one thing that’s negligible.

Disclaimer: This audit report shouldn’t be a safety guarantee, funding recommendation, or an approval of the Recognized Origin Digital Asset undertaking 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.

Back to top button