Farseen0/scar-pairs huggingface dataset:
HIGH severity: Wrong decimals in the calculation of liquidity amount during redemption. **Description:** The contract `SecuritizeRedemption` provides a public function `redeem()` so that investors can redeem their asset (DS Token) for liquidity token. (Stable Coin)
The parameter `_amount` represents the amount of asset to be redeemed and it is in decimals of the asset token.
The...

Vulnerable code:
SecuritizeRedemption.sol
77:     function redeem(uint256 _amount) whenNotPaused external override {
78:         uint256 rate = navProvider.rate();
79:         require(rate != 0, "Rate should be defined");
80:         require(asset.balanceOf(msg.sender) >= _amount, "Redeemer has not enough balance");
81:         require(address(liquidityProvider) != address(0), "Liquidity provider should be defined");
82:         require(liquidityProvider.availableLiquidity() >= _amount, "Not enough liquidity");
83:
84:         ERC20 stableCoin = ERC20(address(liquidityProvider.liquidityToken()));
85:         uint256 liquidity = _amount * rate / (10 ** stableCoin.decimals());
86:
87:         liquidityProvider.supplyTo(msg.sender, liquidity);
88:         asset.transferFrom(msg.sender, liquidityProvider.recipient(), _amount);
89:
90:         emit RedemptionCompleted(msg.sender, _amount, liquidity, rate);
91:     }

-         uint256 liquidity = _amount * rate / (10 ** stableCoin.decimals());
+         uint256 liquidity = _amount * rate / (10 ** asset.decimals());
HIGH severity: Missing validation on the `msg.sender` in `SecuritizeSwap::buy` function. **Description:** The function `SecuritizeSwap::buy` is supposed to be used by investors to swap `stableCoinToken` for `dsToken` and the key difference from the function `SecuritizeSwap::swap` is that `buy` can be called by anyone while the `swap` function can only be called by `Issuer` or...

Vulnerable code:
SecuritizeSwap.sol
104:     function buy(//@audit-info public call that investors can directly call
105:         string memory _senderInvestorId,
106:         address _investorWallet,
107:         uint256 _stableCoinAmount,
108:         uint256 _blockLimit,
109:         uint256 _issuanceTime
110:     ) public override whenNotPaused {
111:         require(_blockLimit >= block.number, "Transaction too old");
112:         require(stableCoinToken.balanceOf(_investorWallet) >= _stableCoinAmount, "Not enough stable tokens balance");
113:         require(IDSRegistryService(dsToken.getDSService(DS_REGISTRY_SERVICE)).isInvestor(_senderInvestorId), "Investor not registered");
114:         require(navProvider.rate() > 0, "NAV Rate must be greater than 0");
115:         (uint256 dsTokenAmount, uint256 currentNavRate) = calculateDsTokenAmount(_stableCoinAmount);
116:         stableCoinToken.transferFrom(_investorWallet, issuerWallet, _stableCoinAmount);
117:
118:         dsToken.issueTokensCustom(_investorWallet, dsTokenAmount, _issuanceTime, 0, "", 0);
119:
120:         emit Buy(msg.sender, _stableCoinAmount, dsTokenAmount, currentNavRate, _investorWallet);
121:     }
HIGH severity: Wrong validation on the `availableLiquidity` in function `SecuritizeRedemption::redeem()`. **Description:** The function `SecuritizeRedemption::redeem(uint256 _amount)` is supposed to be used by investors to be able to redeem their assets (DSToken) for liquidity (stable coin). The parameter `_amount` is the amount of asset that the caller wants to redeem.

Looking at the implementation,.....

Vulnerable code:
SecuritizeRedemption.sol
77:     function redeem(uint256 _amount) whenNotPaused external override {
78:         uint256 rate = navProvider.rate();
79:         require(rate != 0, "Rate should be defined");
80:         require(asset.balanceOf(msg.sender) >= _amount, "Redeemer has not enough balance");
81:         require(address(liquidityProvider) != address(0), "Liquidity provider should be defined");
82:         require(liquidityProvider.availableLiquidity() >= _amount, "Not enough liquidity");//@audit-issue WRONG
83:
84:         ERC20 stableCoin = ERC20(address(liquidityProvider.liquidityToken()));
85:         uint256 liquidity = _amount * rate / (10 ** stableCoin.decimals());
86:
87:         liquidityProvider.supplyTo(msg.sender, liquidity);
88:         asset.transferFrom(msg.sender, liquidityProvider.recipient(), _amount);
89:
90:         emit RedemptionCompleted(msg.sender, _amount, liquidity, rate);
91:     }
HIGH severity: Missing access control in `CollateralLiquidityProvider::setCollateralProvider`. **Description:** The contract `CollateralLiquidityProvider` relies on `collateralProvider` to pull the collateral and process redemption in liquidity token. This state variable can be changed via the function `setCollateralProvider()` as below.

Looking at the implementation, the function is not...

Vulnerable code:
CollateralLiquidityProvider.sol
79:     function setCollateralProvider(address _collateralProvider) external {//@audit-issue CRITICAL missing access control
80:         collateralProvider = _collateralProvider;
81:     }
HIGH severity: Missing access control in `CollateralLiquidityProvider::setExternalCollateralRedemption`. **Description:** The contract `CollateralLiquidityProvider` relies on `externalCollateralRedemption` to provide liquidity and this state variable can be changed via the function `setExternalCollateralRedemption()` as below.

Looking at the implementation, the function is not protected by any...

Vulnerable code:
CollateralLiquidityProvider.sol
75:     function setExternalCollateralRedemption(address _externalCollateralRedemption) external override {//@audit-issue CRITICAL missing access control
76:         externalCollateralRedemption = IRedemption(_externalCollateralRedemption);
77:     }
MEDIUM severity: No validation check of `_investorWallet` when buying `dsToken`.. **Description:** In the `SecuritizeSwap.buy()` function, there is no validation check to verify if `_investorWallet` has already been added to the wallet list of `_senderInvestorId`, unlike in the `swap()` function. Consequently, users can purchase `dsToken` using wallets that have not been...

Vulnerable code:
File: securitize_dev-securitize-swap-47704357c2da\contracts\swap\SecuritizeSwap.sol

88:         //Check if new wallet should be added
89:         string memory investorWithNewWallet = IDSRegistryService(dsToken.getDSService(DS_REGISTRY_SERVICE)).getInvestor(_newInvestorWallet);
90:         if(CommonUtils.isEmptyString(investorWithNewWallet)) {
91:             IDSRegistryService(dsToken.getDSService(DS_REGISTRY_SERVICE)).addWallet(_newInvestorWallet, _senderInvestorId);
92:         } else {
93:             require(CommonUtils.isEqualString(_senderInvestorId, investorWithNewWallet), "Wallet does not belong to investor");
94:         }
MEDIUM severity: Missing validation of `msg.value` in the function `SecuritizeSwap::executePreApprovedTransaction()`. **Description:** `SecuritizeSwap` allows anyone to execute pre-approved transactions using the function `SecuritizeSwap::executePreApprovedTransaction`. This function validates that the protocol is not paused and the length of `params` parameter is two and then calls...

Vulnerable code:
SecuritizeSwap.sol
191:         uint256 value = _params[0];
192:         uint256 gasLimit = _params[1];
193:         assembly {
194:             let ptr := add(_data, 0x20)
195:             let result := call(gasLimit, _destination, value, ptr, mload(_data), 0, 0)
196:             let size := returndatasize()
197:             returndatacopy(ptr, 0, size)
198:             switch result
199:             case 0 {
200:                 revert(ptr, size)
201:             }
202:             default {
203:                 return(ptr, size)
204:             }
205:         }
LOW severity: Precision loss in the `SecuritizeSwap.calculateDsTokenAmount()` function.. **Description:** When `stableCoinDecimals` is greater than `dsTokenDecimals`, the calculation is divided into two parts: dividing by `(10 ** (stableCoinDecimals - dsTokenDecimals))` at `L242` and then multiplying by `10 ** stableCoinDecimals` at `L245`. This can result in a loss of...

Vulnerable code:
if (stableCoinDecimals <= dsTokenDecimals) {
            adjustedStableCoinAmount = _stableCoinAmount * (10 ** (dsTokenDecimals - stableCoinDecimals));
        } else {
242         adjustedStableCoinAmount = _stableCoinAmount / (10 ** (stableCoinDecimals - dsTokenDecimals));
        }
        // The InternalNavSecuritizeImplementation uses rate expressed with same number of decimals as stableCoin
245     uint256 dsTokenAmount = adjustedStableCoinAmount * 10 ** stableCoinDecimals / currentNavRate;

dsTokenAmount = _stableCoinAmount * 10 ** dsTokenDecimals / currentNavRate;

function calculateDsTokenAmount(uint256 _stableCoinAmount) internal view returns (uint256, uint256) {
        uint256 stableCoinDecimals = ERC20(address(stableCoinToken)).decimals();
        uint256 dsTokenDecimals = ERC20(address(dsToken)).decimals();
        uint256 currentNavRate = navProvider.rate();

-       uint256 adjustedStableCoinAmount;
-       if (stableCoinDecimals <= dsTokenDecimals) {
-           adjustedStableCoinAmount = _stableCoinAmount * (10 ** (dsTokenDecimals - stableCoinDecimals));
-       } else {
-           adjustedStableCoinAmount = _stableCoinAmount / (10 ** (stableCoinDecimals - dsTokenDecimals));
-       }
-       // The InternalNavSecuritizeImplementation uses rate expressed with same number of decimals as stableCoin
-       uint256 dsTokenAmount = adjustedStableCoinAmount * 10 ** stableCoinDecimals / currentNavRate;

+       uint256 dsTokenAmount = _stableCoinAmount * 10 ** dsTokenDecimals / currentNavRate;

        return (dsTokenAmount, currentNavRate);
    }
HIGH severity: The `SecuritizeRedemption.updateLiquidityProvider` function emits the wrong information.. **Description:** The `SecuritizeRedemption.updateLiquidityProvider` function emits the information that `oldProvider` is updated to `liquidityProvider`.
But the function allocates `_liquidityProvider` to `oldProvider` instead of `liquidityProvider`

In the `updateLiquidityProvider` function, it...

Vulnerable code:
File: securitize_dev-bc-redemption-sc-32e23d5318be\contracts\redemption\SecuritizeRedemption.sol
65:     function updateLiquidityProvider(address _liquidityProvider) onlyOwner external override {
66:         address oldProvider = address(_liquidityProvider);
67:         liquidityProvider = ILiquidityProvider(_liquidityProvider);
68:         emit LiquidityProviderUpdated(oldProvider, address(liquidityProvider));
69:     }
LOW severity: Missing slippage control for investors during the buy. **Description:** The protocol allows investors to buy `dsToken` with `stableCoinToken` using the function `SecuritizeSwap::buy()`.
The function calculates the amount of `dsToken` to send to the caller based on the `navProvider.rate()` and there is no way for an investor to predict what rate will...

Vulnerable code:
SecuritizeSwap.sol
104:     function buy(
105:         string memory _senderInvestorId,
106:         address _investorWallet,
107:         uint256 _stableCoinAmount,
108:         uint256 _blockLimit,
109:         uint256 _issuanceTime
110:     ) public override whenNotPaused {
111:         require(_blockLimit >= block.number, "Transaction too old");//@audit-ok
112:         require(stableCoinToken.balanceOf(_investorWallet) >= _stableCoinAmount, "Not enough stable tokens balance");
113:         require(IDSRegistryService(dsToken.getDSService(DS_REGISTRY_SERVICE)).isInvestor(_senderInvestorId), "Investor not registered");
114:         require(navProvider.rate() > 0, "NAV Rate must be greater than 0");
115:         (uint256 dsTokenAmount, uint256 currentNavRate) = calculateDsTokenAmount(_stableCoinAmount);
116:         stableCoinToken.transferFrom(_investorWallet, issuerWallet, _stableCoinAmount);
117:
118:         dsToken.issueTokensCustom(_investorWallet, dsTokenAmount, _issuanceTime, 0, "", 0);//@audit-issue missing protection against change in the NAV rate
119:
120:         emit Buy(msg.sender, _stableCoinAmount, dsTokenAmount, currentNavRate, _investorWallet);
121:     }
122:
LOW severity: Missing input validation in `CollateralLiquidityProvider::setExternalCollateralRedemption`. **Description:** _Note that this finding assumes the other finding about access control is mitigated_.
The contract `CollateralLiquidityProvider` relies on `externalCollateralRedemption` to process redemption.
The function `supplyTo()`  is the core function of this contract.

Looking at the L69...

Vulnerable code:
CollateralLiquidityProvider.sol
61:     function supplyTo(address _redeemer, uint256 _amount) whenNotPaused onlySecuritizeRedemption public override {
62:         //take collateral funds from collateral provider
63:         IERC20(externalCollateralRedemption.asset()).transferFrom(collateralProvider, address(this), _amount);
64:
65:         //approve external redemption
66:         IERC20(externalCollateralRedemption.asset()).approve(address(externalCollateralRedemption), _amount);
67:
68:         //get liquidity
69:         externalCollateralRedemption.redeem(_amount);
70:
71:         //supply _redeemer
72:         liquidityToken.transfer(_redeemer, _amount);
73:     }
HIGH severity: Impossible to reveal any drop boxes linked to codes once enough codes have been associated such that `remainingBoxesAmount == 0`. **Description:** In `DropBox::associateOneTimeCodeToAddress`, the `boxAmount` linked to allocated codes is deducted from `remainingBoxesAmount` storage:

Then in `DropBox::revealDropBoxes` when the user attempts to reveal the boxes previously linked with a code, if all the codes have been...

Vulnerable code:
// Decrease the amount of boxes remaining to mint, since they are now associated to be minted
remainingBoxesAmount -= boxAmount;

// @audit read current `remainingBoxesAmount` from storage
// the box amount linked with this code has already been
// previously deducted in `associateOneTimeCodeToAddress`
uint256 remainingBoxesAmountCache = remainingBoxesAmount;

// @audit since the box amount linked with this code has
// already been deducted from `remainingBoxesAmount`, if
// all codes have been associated, even though not all codes have been
// revealed attempting to reveal any codes will erroneously revert since
// remainingBoxesAmountCache == 0
if (remainingBoxesAmountCache == 0) revert NoMoreBoxesToMint();

// @audit even if the previous check was removed, the transaction would
// still revert here since remainingBoxesAmountCache == 0 but
// oneTimeCodeData.boxAmount > 0
if (remainingBoxesAmountCache < oneTimeCodeData.boxAmount) revert NoMoreBoxesToMint();

dropBox = new DropBoxMock(
      address(users.operatorOwner),
      "https://api.example.com/",
      "https://api.example.com/contract",
      address(earnmERC20Token),
      address(users.apiAddress),
      [10_000_000, 1_000_000, 100_000, 10_000, 2500, 750],
-     [1, 10, 100, 1000, 2000, 6889],
+     [uint16(1), 0, 0, 0, 0, 0],
      ["Mythical Box", "Legendary Box", "Epic Box", "Rare Box", "Uncommon Box", "Common Box"],
      address(vrfHandler)
    );
MEDIUM severity: `DropBox::tokenURI` returns data for non-existent `tokenId` violating `EIP721`. **Description:** `DropBox::tokenURI` overrides OpenZeppelin's default implementation with the following code:

This implementation does not check that `tokenId` exists and hence will return data for non-existent `tokenId`. This is in contrast to OpenZeppelin's...

Vulnerable code:
function tokenURI(uint256 tokenId) public view override returns (string memory) {
  return string(abi.encodePacked(bytes(_baseTokenURI), Strings.toString(tokenId), ".json"));
}

/// @notice A distinct Uniform Resource Identifier (URI) for a given asset.
/// @dev Throws if `_tokenId` is not a valid NFT. URIs are defined in RFC
///  3986. The URI may point to a JSON file that conforms to the "ERC721
///  Metadata JSON Schema".
function tokenURI(uint256 _tokenId) external view returns (string);

function tokenURI(uint256 tokenId) public view override returns (string memory) {
+ _requireOwned(tokenId);
  return string(abi.encodePacked(bytes(_baseTokenURI), Strings.toString(tokenId), ".json"));
}
LOW severity: Protocol can collect less or more fees than expected due to constant `claimFee` per transaction regardless of how many boxes are claimed. **Description:** `DropBox::claimDropBoxes` allows users to pass an array of `boxIds` to claim per transaction while charging a constant `claimFee` per transaction:

This means that (not including gas fees):
* claiming 1 box costs the same as claiming 100 boxes, if done in the 1 transaction
*...

Vulnerable code:
// Require that the user sends a fee of "claimFee" amount
if (msg.value != claimFee) revert InvalidClaimFee();

// Require that the user sends a fee of "claimFee" amount per box
if (msg.value != claimFee * _boxIds.length) revert InvalidClaimFee();
HIGH severity: Use inheritance order from most "base-like" to "most-derived" so `super` will call correct parent function in `DropBox::supportsInterface`. **Description:** Use inheritance order from most "base-like" to "most-derived"; this is considered good practice and can be important since Solidity [searches](https://solidity-by-example.org/inheritance/) for parent functions from right to left.

`DropBox` inherits and overrides the following...

Vulnerable code:
function supportsInterface(bytes4 interfaceId) public view virtual override(ERC721C, ERC2981) returns (bool) {
    return super.supportsInterface(interfaceId);
  }

contract DropBox is OwnableBasic, ERC721C, BasicRoyalties, ReentrancyGuard, DropBoxFractalProtocol, IVRFHandlerReceiver {

/**
     * @notice Indicates whether the contract implements the specified interface.
     * @dev Overrides supportsInterface in ERC165.
     * @param interfaceId The interface id
     * @return true if the contract implements the specified interface, false otherwise
     */
    function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) {
        return
        interfaceId == type(ICreatorToken).interfaceId ||
        interfaceId == type(ICreatorTokenLegacy).interfaceId ||
        super.supportsInterface(interfaceId);
    }
HIGH severity: Avoid floating pragma unless creating libraries. **Description:** Per [SWC-103](https://swcregistry.io/docs/SWC-103/) compiler versions in pragmas should be fixed unless creating libraries. Choose a specific compiler version to use for development, testing and deployment, eg:

**Mode:**
Fixed in commit...

Vulnerable code:
- pragma solidity ^0.8.19;
+ pragma solidity 0.8.19;
HIGH severity: Use constant `TIER_IDS_LENGTH` in constructor inputs. **Description:** Use constant `TIER_IDS_LENGTH` in constructor inputs:

**Mode:**
Fixed in commit [d8af391](https://github.com/Earnft/dropbox-smart-contracts/commit/d8af391035ca785e2c280de4577f79de2ffda59d).

**Cyfrin:** Verified.

Vulnerable code:
// DropBox:
  constructor(
    uint256 _subscriptionId,
    address _vrfCoordinator,
    address _feesReceiverAddress,
    string memory __baseTokenURI,
    string memory __contractURI,
    address _earmmERC20Token,
    bytes32 _keyHash,
    address _apiAddress,
    uint24[TIER_IDS_LENGTH] memory _tierTokens,
    uint16[TIER_IDS_LENGTH] memory _tierMaxBoxes,
    string[TIER_IDS_LENGTH] memory _tierNames
  )

// DropBoxFractalProtocol:
  constructor(uint24[TIER_IDS_LENGTH] memory tierTokens,
              uint16[TIER_IDS_LENGTH] memory tierMaxBoxes,
              string[TIER_IDS_LENGTH] memory tierNames) {
LOW severity: `VRFHandler` shouldn't inherit from `ConfirmedOwner` since it inherits from `VRFConsumerBaseV2Plus` which already inherits from `ConfirmedOwner`. **Description:** `VRFHandler` shouldn't inherit from `ConfirmedOwner` since it inherits from `VRFConsumerBaseV2Plus` which already [inherits](https://github.com/smartcontractkit/chainlink/blob/develop/contracts/src/v0.8/vrf/dev/VRFConsumerBaseV2Plus.sol#L101) from...

Vulnerable code:
- contract VRFHandler is ConfirmedOwner, VRFConsumerBaseV2Plus, IVRFHandler {
+ contract VRFHandler is VRFConsumerBaseV2Plus, IVRFHandler {
HIGH severity: Use named imports instead of importing the entire namespace. **Description:** Use named imports as they offer a number of [advantages](https://ethereum.stackexchange.com/a/117173) compared to importing the entire namespace.

File: `DropBox.sol`

File: `VRFHandler.sol`

**Mode:**
Fixed in commit...

Vulnerable code:
// DropBoxFractalProtocol
import {DropBoxFractalProtocol} from "./DropBoxFractalProtocol.sol";

// VRF Handler Interface
import {IVRFHandler} from "./IVRFHandler.sol";
import {IVRFHandlerReceiver} from "./IVRFHandlerReceiver.sol";

// LimitBreak implementations of ERC721C and Programmable Royalties
import {OwnableBasic, OwnablePermissions} from "@limitbreak/creator-token-standards/src/access/OwnableBasic.sol";
import {ERC721C, ERC721OpenZeppelin} from "@limitbreak/creator-token-standards/src/erc721c/ERC721C.sol";
import {BasicRoyalties, ERC2981} from "@limitbreak/creator-token-standards/src/programmable-royalties/BasicRoyalties.sol";

// OpenZeppelin
import {ReentrancyGuard} from "@openzeppelin/contracts/security/ReentrancyGuard.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";

// VRF Handler Interface
import {IVRFHandler} from "./IVRFHandler.sol";
import {IVRFHandlerReceiver} from "./IVRFHandlerReceiver.sol";

// Chainlink
import {VRFConsumerBaseV2Plus} from "@chainlink/contracts/src/v0.8/vrf/dev/VRFConsumerBaseV2Plus.sol";
import {IVRFCoordinatorV2Plus} from "@chainlink/contracts/src/v0.8/vrf/dev/interfaces/IVRFCoordinatorV2Plus.sol";
import {VRFV2PlusClient} from "@chainlink/contracts/src/v0.8/vrf/dev/libraries/VRFV2PlusClient.sol";
HIGH severity: Remove `< 0` checks for unsigned variables since they can't be negative. **Description:** Remove `< 0` checks for unsigned variables since they can't be negative:

File: `src/DropBox.sol`

**Mode:**
Fixed in commit [ccc358a](https://github.com/Earnft/dropbox-smart-contracts/commit/ccc358a88e27ed08c85a37c7770af4dea24f9155).

**Cyfrin:** Verified.

Vulnerable code:
538:      if (_randomWords[i] <= 0) revert InvalidRandomWord();
HIGH severity: Enforce ascending order for boxIds to implement more efficient duplicate prevention in `DropBox::claimDropBoxes`. **Description:** Enforce ascending order for boxIds to implements more efficient duplicate prevention in `DropBox::claimDropBoxes`:

**Mode:**
Fixed in commit [756fc82](https://github.com/Earnft/dropbox-smart-contracts/commit/756fc827eb5265c09e7e1eca8c7a55832e58d1f2).

**Cyfrin:** Verified.

Vulnerable code:
-      for (uint256 j = i + 1; j < _boxIds.length; j++) {
-        if (_boxIds[i] == _boxIds[j]) revert BadRequest();
-      }
+     if(i > 0 && _boxIds[i-1] >= _boxIds[i]) revert BadRequest();