The function `approveCollection()` from contract `CollectionsRegistry` does not have any access control mechanism that limits which users can call it.

This lack of control allows any bad actor to approve their malicious contracts.

Missing Access Control

The contract `CollectionsRegistry` only can approve collections. In the case of a user who approves a malicious contract, there is no mechanism to revoke the approval from that contract.

This lack of mechanism would have the consequence of having a malicious contract permanently approved without the possibility of removing it. This scenario could lead to damage to the team's reputation.

Contract Lacks a Mechanism To Revoke Malicious Collections

In the contract `DropRewardEscrow` the role `_owner` has authority over the functions shown in the diagram below.

![](https://accelerator-tasks-prod.s3.amazonaws.com/e056d7a958774eb799300ad4b071b59f/diagrams/centralization_DropRewardEscrow__owner.svg)

In the contract `DropRewardEscrow` the role `contractOwner` has authority over the functions shown in the diagram below.

![](https://accelerator-tasks-prod.s3.amazonaws.com/e056d7a958774eb799300ad4b071b59f/diagrams/centralization_DropRewardEscrow_contractOwner.svg)

In the contract `DroppingNowMarketplace` the role `_owner` has authority over the functions shown in the diagram below.

![](https://accelerator-tasks-prod.s3.amazonaws.com/e056d7a958774eb799300ad4b071b59f/diagrams/centralization_DroppingNowMarketplace__owner.svg)

In the contract `DroppingNowMarketplace` the role `seller` has authority over the functions shown in the diagram below.

![](https://accelerator-tasks-prod.s3.amazonaws.com/e056d7a958774eb799300ad4b071b59f/diagrams/centralization_DroppingNowMarketplace_seller.svg)

In the contract `PriceCalculatorManager` the role `_owner` has authority over the functions shown in the diagram below.

![](https://accelerator-tasks-prod.s3.amazonaws.com/e056d7a958774eb799300ad4b071b59f/diagrams/centralization_PriceCalculatorManager__owner.svg)

In the contract `SimpleERC1155Token` the role `_owner` has authority over the functions shown in the diagram below.

![](https://accelerator-tasks-prod.s3.amazonaws.com/e056d7a958774eb799300ad4b071b59f/diagrams/centralization_SimpleERC1155Token__owner.svg)

In the contract `SimpleERC721Token` the role `_owner` has authority over the functions shown in the diagram below.

![](https://accelerator-tasks-prod.s3.amazonaws.com/e056d7a958774eb799300ad4b071b59f/diagrams/centralization_SimpleERC721Token__owner.svg)

In the contract `TokenManagerMarketplace` the role `_owner` has authority over the functions shown in the diagram below.

![](https://accelerator-tasks-prod.s3.amazonaws.com/e056d7a958774eb799300ad4b071b59f/diagrams/centralization_TokenManagerMarketplace__owner.svg)

In the contract `TokenManagerSelector` the role `_owner` has authority over the functions shown in the diagram below.

![](https://accelerator-tasks-prod.s3.amazonaws.com/e056d7a958774eb799300ad4b071b59f/diagrams/centralization_TokenManagerSelector__owner.svg)

Any compromise to these privileged accounts may allow the hacker to take advantage of these authorities and access sensitive functionalities.

Centralization Related Risks

The linked statements make use of a parameter that was not validated before its usage.

In the case of the variables `dropperFeeValue` and `marketplaceFeeValue` from the constructor of the contract `DroppingNowMarketplace`, there is no check that prevents the values from being greater than `10.000`.

In the case of the variable `contractUri` from the function `_setContractURI()` from the contract `DropperToken`, there is no check that prevents the variable from being an empty string.

In the case of the variable `newUri` from the function `setURI()` from contract `DropperToken`, there is no check that prevents the value from being an empty string. A similar case happens to the variable `contractUri` from the function `setContractURI()` from the same contract. 

Missing Input Validation

The function `viewAllowedMarketplaces()` returns the value `cursor + length`. In the case where the cursor is bigger than the size of the array, the variable `length` will be zero, and the `allowedMarketplaces` array will be empty, but the second return value will be the cursor value rather than the size of the `allowedMarketplaces` array.

Incorrect Return Value

After [EIP-1884](https://eips.ethereum.org/EIPS/eip-1884) was included in the Istanbul hard fork, it is not recommended to use `.transfer()` or `.send()` for transferring ether as these functions have a hard-coded value for gas costs making them obsolete as they are forwarding a fixed amount of gas, specifically `2300`. This can cause issues in case the linked statements are meant to be able to transfer funds to other contracts instead of EOAs.

Usage of `transfer()` for sending Ether

Addresses should be checked before assignment usage to make sure they are not zero addresses.

Missing Zero Address Validation

The linked statements do not handle the return value of the external functions they are calling.

Unused Return Value

The linked magic numbers should be set as `constant` and `internal` contract-level variables with a self-explanatory variable name as well as accompanying comments when necessary. This type of declaration is functionally equivalent to the current implementation as `constant` variables that are `internal` or `private` are simply replaced in the codebase with their literal value.

In the case of the `TokenManagerSelector` contract, the usage of the zero address can be replaced by the private variable `NO_MANAGER` to improve the code's readability.

Magic Numbers

The contracts `PriceCalculatorManager` and `TokenManagerMarketplace` are almost identical. They both manage assets that are added, removed, and listed.

A base contract could be created with this shared functionality to avoid duplicating the code.

`PriceCalculatorManager` Contract Is Almost Identical To `TokenManagerMarketplace`

There should always be events emitted in the sensitive functions that are controlled by centralization roles.

Missing Emit Events

`public` functions that are never called by the contract could be declared as `external`. `external` functions are more efficient than `public` functions.

Improper Usage of `public` and `external` Type

In the project's [documentation](https://droppingnow.gitbook.io/product-docs/welcome/about-droppingnow/why-dutch-auctions), it states that the auction's creator can't cancel its listing.

However, this is possible via the `cancelSingleAuction()` function, which only requires that the one calling it is the auction's seller.

Inconsistent Function With Documentation `cancelSingleAuction()`

The local variable `amount` from the function `_getDropRewardAmounts()` from the contract  `DroppingNowMarketplace` is an array with hard-coded values which is returned to the user.

As this variable does not change and it defines a parameter inside the contract, it should be a contract's `public` variable.

Local Variable Should Be State Variable

The variable `ownerHasCorrectAddressAndApproved` from function `_saleReward()` from contract `DroppingNowMarketplace` has a redundant value when it is assigned.

The variable is assigned only if the variable `isApproved` is true. If this happens, the new value will be `isApproved && owner != address(0)`. There is no need to make an `and` operation as the value `isApproved` is already true.

Redundant Assignment For `ownerHasCorrectAddressAndApproved`

The linked `require` statements have a condition that checks whether a variable of type `uint` is greater or equal to zero.

This check is redundant for these types of variables as they are non-negative integers.

Redundant Condition

The functions `buySingleAuction()` and `buyBundleAuction()` from contract `DroppingNowMarketplace` are almost identical. The shared functionality can be placed into a separate `internal` or `private` function to improve the code's readability.

Code Duplication

The function `tryAddMintable()` from contract `DropperToken` doesn't have a return value. This lack of feedback makes it difficult to tell if the function added or not the token address.

A similar case occurs with the function `tryAddMintableBatch()`.

Missing Return Value

The function `isPriceAllowed()` makes the same multiplication twice. The first one is to check if the operation is safe, while the last one is part of the calculation of the `nextDayPrice`.

The calculation of `nextDayPrice` could use the second return value of SafeMath's `tryMul()`, which is the result of the operation, to save the cost of multiplication and the default security checks.

Price Recalculation

The function `_calculate()` from contract `PriceCalculatorDrop25PerDay` makes a `for` loop to calculate the current price devaluation given its starting price and the number of days.

The `for` loop can be replaced with the direct calculation of `(currentPrice * (3 ** daysGone)) >> (2 * daysGone);`. Being, `>>` the shift operator.

This calculation is much cheaper as it doesn't depend on the number of iterations. After making a few tests, we saw that if we call the `for` loop, it had a gas cost of `12765` when the recommended version only used `3159`. The values used were: `currentPrice = 748` and `daysGone = 15`.

`currentPrice` Loop Calculation Instead Of Direct Calculation

The function `supportsInterface()` from contract `SimpleERC20Token` has the condition `interfaceId == type(IERC20).interfaceId` repeated twice.

Given the context, it seems that, on the duplicated line, the interface used should be `IERC165`.

Repeated Condition on `supportsInterface()`

The function `removeMarketplace()` from contract `TokenManagerMarketplace` has a `require` statement which has an unclear error message "TokenManagerMarketplace: Not allowed". This gives the user the intuition that the action is not allowed, but instead it refers to that the marketplace was not allowed in the first place.

Unclear Error Message

The functions `deposit()` and `withdraw()` from contract `TokenManagerERC721` return the value zero. This value is inconsistent with the contract `TokenManagerERC1155`, where the return value is the number of tokens transferred.

Inconsistent Return Values

The variable `tokenManagerSelectorForTokenAddress` from contract `TokenManagerSelector` is a bit confusing.

Confusing Variable Name `tokenManagerSelectorForTokenAddress`

CryptoSlam - DroppingNow

PriceCalculatorDrop25PerDay.sol

TokenManagerERC1155.sol

ICollectionsRegistry.sol

IDropperToken.sol

SimpleERC20Token.sol

ITokenManagerSelector.sol

HashHelper.sol

DropperToken.sol

CollectionsRegistry.sol

IPriceCalculatorManager.sol

DroppingNowToken.sol

ITokenManager.sol

SimpleERC721Token.sol

TokenManagerSelector.sol

DropRewardEscrow.sol

PriceCalculatorManager.sol

DroppingNowMarketplace.sol

IPriceCalculator.sol

SimpleERC1155Token.sol

TokenManagerMarketplace.sol

TokenManagerERC721.sol

IDroppingNowToken.sol

CryptoSlam

Trump’s NFT Digital Trading Cards Bring in Millions of Dollars—and GOP Ridicule - WSJ

Hermès Wins Case Against Artist Who Sold NFTs of Birkin Bags

Candy Digital NFT shop lays off large number of employees amid market deterioration

Element, the multi-chain aggregation NFT marketplace, has reached a volume exceeded 100 million USD. | NewsBTC

DefiLlama founder moves to NFT lending to tackle liquidity constraints

Critics can’t stop NFTs from becoming a mainstay of daily life

Cryptocurrency Tax 2022-2023: Your Crypto Tax Bill With Bitcoin, NFT Losses

Bridge Champ Introduces NFT Badges and Crypto Rewards, New Roadmap Says

Coinbase says Apple forced it to remove NFT wallet function

Gods Unchained Celebrates Festive Season with Winter-Themed NFTs

First Mover Americas: Bitcoin ‘Ordinals’ Boom Prompts NFT Activity Surge

Cryptocurrency Tax 2022-2023: Your Crypto Tax Bill With Bitcoin, NFT Losses - WSJ

Fun new NFT use case just dropped! - The Verge

Gaming titan Square Enix plots NFT game on Polygon

Why This Crypto Bill Advancing In Illinois is Worrying For DeFi, NFTs, Gaming, and Blockchain

NFT startup Candy Digital raised $38.5 million at the start of this year

NFT Marketplace Blur Co-Founder ‘Pacman’ Reveals Identity

How to Handle Cryptocurrencies and NFTs in Your Estate Planning - WSJ

Defi Llama founder moves to NFT lending to tackle liquidity constraints

Axie Infinity NFT Game Loses 62% of Players in One Year

China Launches NFT Marketplace, Crypto Daily TV 30/12/2022

Want To Know How CertiK Performs An Audit?

Want To Secure Your Wallet, Exchange or Dapp? Check Out Our Web 3 Pentesting

CryptoSlam

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

24

0

24

0

0

Products & Technology

Bug Bounty

Smart Contract Audit

SkyTrace

L1 Chain Audit

Incident Response

Skynet

KYC

Advisory Services

Penetration Testing

Sky Harbor

Formal Verification

Featured Ecosystems

BNB Chain

Ethereum

Avalanche

Solana

Algorand

NEAR

Cosmos

Polygon

Aptos

Company

About

Testimonials

Partnership

We're Hiring

Disclaimer

Resources

CryptoSlam

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

24

0

24

0

0