Farseen0/scar-pairs huggingface dataset:
HIGH severity: Malicious user can easily make the protocol revert on every `USDT` swap on Uniswap. **Likelihood:**
High, attack can easily be done and it exploits a well-known attack vector of `USDT`

**Impact:**
Medium, because the protocol will not work with only one ERC20 token, but it is a widely used one

**Description**

A malicious user can get the `ArbitrumSwaps` contract to revert on...

Vulnerable code:
TransferHelper.safeApprove(
            swapParams.token1, address(swapRouter), IERC20(swapParams.token1).balanceOf(address(this))
        );
LOW severity: The `gasLeft()` after gas-limited external call might not be enough to complete the transaction. In `StargateArbitrum::sgReceive` we have the following piece of code

Now if the `arbitrumSwaps` call took up all of the gas it is possible that there is not enough gas left for the `safeTransfer` call, as well for the code below it. Consider a different approach, that will check `gasleft()` and...

Vulnerable code:
try IArbitrumSwaps(payable(address(this))).arbitrumSwaps{gas: 200000}(steps, data) {}
        catch (bytes memory) {
            IERC20(_token).safeTransfer(to, amountLD);
            failed = true;
        }
HIGH severity: Value of `leaf` argument when calling `addReserve` is hardcoded incorrectly. **Impact:**
High, because liquidity won't be returned to the LiquidityTree

**Likelihood:**
High, because the incorrect value is hardcoded and can't be changed

**Description**

In `BetExpress::resolvePayout` we can see the following code:

where the last argument is 0 sent as a value for the...

Vulnerable code:
uint128 reward = lp.addReserve(
    0,
    fullPayout - amount,
    fullPayout - payout,
    0
    );
MEDIUM severity: Tokens with a no-op fallback function can be used to steal the ETH balance of `LP`. **Impact:**
High, because it can lead to stolen funds from the protocol

**Likelihood:**
Low, as it requires a token with a fallback function but without a `withdraw` function

**Description**

In `LP::withdrawPayout` we have the following code:

Now imagine the following scenario:

1. The `token`.....

Vulnerable code:
if (isNative) {
    IWNative(token).withdraw(amount);
    TransferHelper.safeTransferETH(account, amount);
} else {
    TransferHelper.safeTransfer(token, account, amount);
}
HIGH severity: Users can split a token to more fractions than the `units` held at `tokenID`. **Impact**
High, as it breaks an important protocol invariant

**Likelihood**
High, as those types of issues are common and are easily exploitable

**Description**

The `_splitValue` method in `SemiFungible1155` does not follow the Checks-Effects-Interactions pattern and it calls `_mintBatch` from.....

Vulnerable code:
-_mintBatch(_account, toIDs, amounts, "");
-
-tokenValues[_tokenID] = valueLeft;
+tokenValues[_tokenID] = valueLeft;
+
+_mintBatch(_account, toIDs, amounts, "");
HIGH severity: Calling `splitValue` when token index is not the latest will overwrite other claims' storage. **Impact:**
High, as it can lead to loss of units for an account without any action on his side

**Likelihood:**
Medium, because it can happen only with a token that has a non-latest index

**Description**

The logic in `_splitValue` is flawed here:

Let's look at the following scenario:

1. Alice.....

Vulnerable code:
uint256 currentID = _tokenID;
...
toIDs[i] = ++currentID;
...
for (uint256 i; i < len; ) {
    valueLeft -= values[i];

    tokenValues[toIDs[i]] = values[i];

    unchecked {
        ++i;
    }
}
...
_mintBatch(_account, toIDs, amounts, "");

- maxIndex[_typeID] += len;
...
- toIDs[i] = ++currentID;
+ toIDs[i] = _typeID + ++maxIndex[typeID];
HIGH severity: It's impossible for a user to claim his rewards, as `claimReward` will never send out `USDC`. **Impact:**
High, because users will never receive rewards from the contract

**Likelihood:**
High, because the code just uses the ERC20 API incorrectly

**Description**

The `claimReward` method should be used by a staker to receive `USDC` rewards for his locked NFTs. This won't ever work, as the.....

Vulnerable code:
USDc.transferFrom(msg.sender, address(this), claimableRewards);

- USDc.transferFrom(msg.sender, address(this), claimableRewards);
+ USDc.transfer(msg.sender, claimableRewards);
HIGH severity: Calculation for `owedAmount` will round down to zero. **Impact:**
High, as this will result in 0 claimable rewards for users when they should have been able to claim some

**Likelihood:**
High, as this will happen any time the user's share is smaller than the pool's cached global share, which is almost always

**Description**

The `claimCalculation`...

Vulnerable code:
owedAmount = (currentShareRaw / pool[_poolNumber].currentGlobalShare) * pool[_poolNumber].value;

- owedAmount = (currentShareRaw / pool[_poolNumber].currentGlobalShare) * pool[_poolNumber].value;
+ owedAmount = currentShareRaw * pool[_poolNumber].value / pool[_poolNumber].currentGlobalShare;
HIGH severity: Wrong check in `claimCalculation` will result in less rewards received for users. **Impact:**
High, as this will lead to a monetary loss for users

**Likelihood:**
Medium, as it happens only for the pool with an ID of 1

**Description**

The first `if` statement in `claimCalculation` checks `if (_poolNumber == 1)` and does not factor in any inflation for that particular pool....

Vulnerable code:
- if (_poolNumber == 1) {
+ if (_poolNumber == 0) {
LOW severity: Unexpected behavior if user stakes in the same block as when the first pool is created. If there is only 1 rewards pool and a user has staked in exactly the same block as when the pool was created, then both

and

will return `true`. The problem is the first check is in the `if` of `getCurrentShareRaw`, while the second one is in the `else if`, and depending on which branch the code...

Vulnerable code:
(pool[pool.length - 1].time) <= timeLizardLocked[_tokenId]
HIGH severity: Contract fails to handle `address(0)` as a native asset for it's zero balance left invariant. **Impact:**
High, as all transactions that use native assets will revert

**Likelihood:**
High, as it is well expected that native assets will be used as a `paymentToken` often

**Description**

The `executePayroll` method has the following code at the end of it:

A few lines above the code looks...

Vulnerable code:
// Check if the contract has any tokens left
for (uint256 i = 0; i < paymentTokens.length; i++) {
    IERC20 erc20 = IERC20(paymentTokens[i]);
    if (erc20.balanceOf(address(this)) > 0) {
        // Revert if the contract has any tokens left
        revert("CS018");
    }
}

if (tokenAddress[i] == address(0)) {
    // Transfer ether
    payable(to[i]).transfer(amount[i]);
    packPayoutNonce(true, payoutNonce[i]);
}
LOW severity: Bid expiration is not constrained. The `enterBid` method is missing input validation on the `expiration` argument so it is possible to enter an already expired bid or a bid that is never expiring. Another thing is that the `updateBids` method allows an already expired bid to be updated. While currently this results only in some...

Vulnerable code:
require(expiration > block.timestamp && expiration < block.timestamp + 365 days);
HIGH severity: Anyone can steal all `HoneyJar` NFTs in `HoneyJarPortal` and exploit its allowances. **Impact:**
High, as it is a value loss for users

**Likelihood:**
High, as it is a common vulnerability and requires no preconditions

**Description**

The `_debitFrom` function in `HoneyJarPortal` is exploitable, as it looks like this:

Since there is no check for the `_from` argument, anyone...

Vulnerable code:
function _debitFrom(address _from, uint16, bytes memory, uint _tokenId) internal override {
    honeyJar.safeTransferFrom(_from, address(this), _tokenId); // Performs the owner & approval checks
}
HIGH severity: Reentrancy allows any user allowed even one free `HoneyJar` mint to mint the max supply for himself for free. **Impact:**
High, as the user will steal all `HoneyJar` NFTs, paying nothing

**Likelihood:**
High, as reentrancy is a very common attack vector and easily exploitable

**Description**

The `claim` method in `HoneyBox` (from its NatSpec) "Allows a player to claim free HoneyJar based on...

Vulnerable code:
_canMintHoneyJar(bundleId_, numClaim); // Validating here because numClaims can change

// If for some reason this fails, GG no honeyJar for you
_mintHoneyJarForBear(msg.sender, bundleId_, numClaim);

claimed[bundleId_] += numClaim;
// Can be combined with "claim" call above, but keeping separate to separate view + modification on gatekeeper
gatekeeper.addClaimed(bundleId_, gateId, numClaim, proof);
HIGH severity: Anyone can mint all NFTs through the public mint before it has even started. **Impact:**
High, as it breaks an important protocol invariant and the way the protocol should work overall

**Likelihood:**
High, as it does not need any preconditions, can be executed easily at the deployment of `HoneyBox`

**Description**

Both the `mekHoneyJarWithERC20` and...

Vulnerable code:
if (slumberParties[bundleId_].publicMintTime > block.timestamp) revert GeneralMintNotOpen(bundleId_);
HIGH severity: Re-requesting randomness from VRF is a security anti-pattern. **Impact:**
High, as the VRF service provider has control over who wins the game

**Likelihood:**
High, as there is an incentive for a VRF provider to exploit this and it is not hard to do from his side

**Description**

The `forceHoneyJarSearch` method is used to "kick off another VRF request",...

Vulnerable code:
Re-requesting randomness is easily detectable on-chain and should be avoided for use cases that want to be considered as using VRFv2 correctly.
HIGH severity: Mint function `mekHoneyJarWithETH` will revert every time. **Impact:**
Medium, as there is an option to mint with ERC20 tokens too

**Likelihood:**
High, as the function will just revert every time

**Description**

The `HoneyBox` contract exposes a way for users to mint `HoneyJar` NFTs with `ETH` in a public sale by the `mekHoneyJarWithETH` method. The...

Vulnerable code:
-    function mekHoneyJarWithETH(uint8 bundleId_, uint256 amount_) external returns (uint256) {
+    function mekHoneyJarWithETH(uint8 bundleId_, uint256 amount_) external payable returns (uint256) {
MEDIUM severity: Possible overflow will break the logic in `HoneyBox`. **Impact:**
High, as bundles storage variables will be overwritten

**Likelihood:**
Low, as it is not expected to add more than 255 bundles

**Description**

In `HoneyBox::addBundle` we have the following code:

The comment is wrong, as it assumes that the cast is safe and will revert if...

Vulnerable code:
uint8 bundleId = uint8(slumberPartyList.length); // Will fail if we have >255 bundles
HIGH severity: Anyone can make new bids always revert after a window expires. **Impact:**
High, as all new bidding will revert until auction ends

**Likelihood:**
High, as anyone can execute the attack without rare preconditions

**Description**

The `fulfillWindow` method is a `public` method that is also called internally. It sets `window.processed` to `true`, which makes.....

Vulnerable code:
if (hasExpired) {
    window = _window[auctionId][windowExpiration(auctionId)];
}

if (window.processed) {
    revert WindowFulfilled();
}
HIGH severity: Successful bidders can lose significant value due to division rounding. **Impact:**
High, as possibly significant value will be lost

**Likelihood:**
High, as it will happen with most bids

**Description**

The `fulfillWindow` method calculates the auction reserves and proceeds after a successful bid in a window. Here is how it accounts it in both the `auctions` and...

Vulnerable code:
_auctions[auctionId].reserves -= volume / price;
_auctions[auctionId].proceeds += volume;

_claims[bidder][auctionId] = abi.encode(refund - volume, claim + (volume / price));
MEDIUM severity: Loss of precision in `scalarPrice` function. **Impact:**
Medium, as the price will not be very far from the expected one

**Likelihood:**
Medium, as it will not always result in big loss of precision

**Description**

In `scalarPrice` there is this code:

Here, when you calculate `x` you divide by `t_r` even though later you multiply `x` by...

Vulnerable code:
uint256 b_18 = 1e18;
uint256 t_mod = t % (t_r - t);
uint256 x = (t + t_mod) * b_18 / t_r;
uint256 y = !isInitialised ? state.price : window.price;

return y - (y * x) / b_18;
MEDIUM severity: Auction won't work correctly with fee-on-transfer & rebasing tokens. **Impact:**
High, as it can lead to a loss of value

**Likelihood:**
Low, as such tokens are not so common

**Description**

The code in `createAuction` does the following:

so it basically caches the expected transferred amount. This will not work if the `reserveToken` has a fee-on-transfer...

Vulnerable code:
IERC20(reserveToken).transferFrom(msg.sender, address(this), reserveAmount);
...
...
state.reserves = reserveAmount;
LOW severity: Auction with `price == 0` can be re-created. The `createAuction` method checks if auction exists with this code

But the method does not check if the `startingOriginPrice` argument had a value of 0 - if it did, then `state.price` would be 0 in the next `createAuction` call. Even though this is not expected to happen, if it does it can lead...

Vulnerable code:
Auction storage state = _auctions[auctionId];

if (state.price != 0) {
    revert AuctionExists();
}

IERC20(reserveToken).transferFrom(msg.sender, address(this), reserveAmount);
HIGH severity: Anyone can move `EURS` tokens that the user allowed `FlorinTreasury` to spend. **Impact:**
High, as funds will be moved from a user's wallet unwillingly

**Likelihood:**
High, as it requires no preconditions and is a common attack vector

**Description**

The `depositEUR` method in `FlorinTreasury` looks like this:

The problem is that the `from` argument is user controlled,.....

Vulnerable code:
function depositEUR(address from, uint256 eurTokens) external whenNotPaused {
    eurTokens = Util.convertDecimals(eurTokens, 18, Util.getERC20Decimals(eurToken));
    SafeERC20Upgradeable.safeTransferFrom(florinToken, from, address(this), eurTokens);
    emit DepositEUR(_msgSender(), from, eurTokens);
}
MEDIUM severity: The Chainlink price feed's input is not properly validated. **Impact:**
High, as it can result in the application working with an incorrect asset price

**Likelihood:**
Low, as Chainlink oracles are mostly reliable, but there has been occurrences of this issue before

**Description**

The code in `LoanVault::getFundingTokenExchangeRate` uses a Chainlink...

Vulnerable code:
(, int256 exchangeRate, , , ) = fundingTokenChainLinkFeeds[fundingToken].latestRoundData();

if (exchangeRate == 0) {
    revert Errors.ZeroExchangeRate();
}

- (, int256 exchangeRate, , , ) = fundingTokenChainLinkFeeds[fundingToken].latestRoundData();
+ (, int256 exchangeRate, , uint256 updatedAt , ) = fundingTokenChainLinkFeeds[fundingToken].latestRoundData();

- if (exchangeRate == 0) {
+ if (exchangeRate <= 0) {
    revert Errors.ZeroExchangeRate();
}

+ if (updatedAt < block.timestamp - 60 * 60 /* 1 hour */) {
+   pause();
+}
LOW severity: Allowance check gives a false sense of security. The `LoanVault::createFundingAttempt` method contains the following allowance check:

But it does not do a `transferFrom` for the `fundingToken` by itself. This means that the call to the method can be back-ran with an allowance revoke transaction. This will invalidate the check and later revert...

Vulnerable code:
if (fundingToken.allowance(_msgSender(), address(this)) < fillableFundingTokenAmount) {
    revert Errors.InsufficientAllowance();
}
HIGH severity: Anyone can cancel another user's scheduled recovery. **Impact:**
High, as important protocol feature can be permanently blocked for a victim user

**Likelihood:**
High, as it has no preconditions and can be exploited by anyone

**Description**

A wallet recovery mechanism in Ambire allows a pre-set account (usually the relayer) to set a new main...

Vulnerable code:
scheduledRecoveries[hash] = block.timestamp + recoveryInfo.timelock;
emit LogRecoveryScheduled(hash, recoveryInfoHash, recoveryKey, currentNonce, block.timestamp, txns);
MEDIUM severity: Invalid signature execution is possible if `address(0)` has non-zero privileges. **Impact:**
High, as anyone will be able to steal all funds from a given wallet

**Likelihood:**
Low, as it requires `address(0)` to have non-zero privileges

**Description**

The problem is similar to [this](https://github.com/code-423n4/2021-10-ambire-findings/issues/13) issue from a previous...

Vulnerable code:
signerKey = SignatureValidator.recoverAddrImpl(hash, signature, true);
require(privileges[signerKey] != bytes32(0), 'INSUFFICIENT_PRIVILEGE');
LOW severity: Not all call paths are anti-bricking protected. Both the `execute` and `executeBySender` call paths have anti-bricking protection that looks like:

and

The problem is that the `fallback` call path is missing this protection, which means not 100% of cases are covered. A possible partial solution is to check if the caller's privileges were...

Vulnerable code:
require(privileges[signerKey] != bytes32(0), 'PRIVILEGE_NOT_DOWNGRADED');

require(privileges[msg.sender] != bytes32(0), 'PRIVILEGE_NOT_DOWNGRADED');
LOW severity: The cached request should be deleted on randomness received. In `RandomProvider::fulfillRandomWords` it is checked that `requests[_requestId].exists == true`. This is correct, but then after the request is fulfilled (as the name of the event emitted in the method) the request value should be deleted from the mapping, so it doesn't exist anymore.

Vulnerable code:
require(requests[_requestId].exists, 'RandomProvider: requestId not found');
        _core.resolveClaimer(requests[_requestId].listingId, randomWords[0]);
        emit RequestFulfilled(_requestId, requests[_requestId].listingId, randomWords);
+       delete requests[_requestId];
HIGH severity: Scaling of `underlyingTokenPrice` will leave funds stuck in protocol. **Impact:**
High, as all deposited funds will be stuck in the protocol

**Likelihood:**
High, as it will always happen

**Description**

The `_getTokenPrices` method in `SwapFacility` has the following code:

This scaling by `1e12` is an error, because most oracle price feeds in Chainlink (and...

Vulnerable code:
underlyingTokenPrice =
    uint256(IOracle(underlyingTokenOracle).latestAnswer()) *
    1e12;
MEDIUM severity: Tokens with a fee-on-transfer mechanism will break the protocol. **Impact:**
High, as some users will lose value

**Likelihood:**
Low, as such tokens are not common

**Description**

The ERC20 logic in `BloomPool` is incompatible with tokens that have a fee-on-transfer mechanism. Such tokens for example is `PAXG`, while `USDT` has a built-in fee-on-transfer...

Vulnerable code:
UNDERLYING_TOKEN.safeTransferFrom(msg.sender, address(this), amount);
MEDIUM severity: Auction tokens with approval race protection or not returning a `bool` on `approve` are incompatible with `VestedCrowdSale`. **Impact:**
High, as sale won't be possible to be settled

**Likelihood:**
Low, as such tokens are rare, but they do exist

**Description**

Some tokens, for example [USDT](https://etherscan.io/token/0xdac17f958d2ee523a2206206994597c13d831ec7) (not a good example because auction tokens have to...

Vulnerable code:
uint256 currentAllowance = sale.auctionToken.allowance(address(this), address(salesVesting[saleId].vestingContract));

//the poor man's `increaseAllowance`
sale.auctionToken.approve(address(salesVesting[saleId].vestingContract), currentAllowance + sale.salesAmount);
MEDIUM severity: Tokens with a fee-on-transfer mechanism will break the protocol. **Impact:**
High, as some users will lose value

**Likelihood:**
Low, as such tokens are not common

**Description**

The ERC20 logic in all crowd sale contracts as well as in `TimelockedToken` is incompatible with tokens that have a fee-on-transfer mechanism. Such tokens for example is `PAXG`,...

Vulnerable code:
sale.auctionToken.safeTransferFrom(msg.sender, address(this), sale.salesAmount);
LOW severity: Usage of `address payable`'s `send` method is discouraged. The `send` method of `address payable` is deprecated as if the callee has a `receive` or `fallback` function that takes up more than 2300 gas then the method will revert. Since there are many smart wallets or multi-sig wallets that take more than 2300 gas on native asset receival, it is...

Vulnerable code:
-require(payable(_msgSender()).send(address(this).balance), "transfer failed");
+msgSender().call{value: address(this).balance}("");
HIGH severity: State is not properly updated on , leading to stuck prize NFTs. **Impact:**
High, as winning prize NFTs won't be claimable

**Likelihood:**
High, as functionality is never working correctly

**Description**

The `addToParty` method in `HibernationDen` does not send a cross-chain message, even though it pushes sleepers into the `party.sleepoors` array on L1....

Vulnerable code:
if (party.numUsed == party.sleepoors.length) revert PartyAlreadyWoke(bundleId_);
HIGH severity: Anyone can burn any NFT in the `HoneyJarPortal`. **Impact:**
High, because the NFTs won't be retrievable from the portal anymore

**Likelihood:**
High, because it does not require any preconditions to be exploited

**Description**

The `_debitFrom` method in `HoneyJarPortal` allows burning of any `HoneyJar` NFT, given its ID. This method is...

Vulnerable code:
if (_from != _msgSender()) revert OwnerNotCaller();
MEDIUM severity: The `fulfillRandomWords` method might revert with out of gas error. **Impact:**
High, as randomness won't be fulfilled

**Likelihood:**
Low, as it requires misconfiguration of gas

**Description**

The `fulfillRandomWords` method in `HibernationDen` calls the internal `_setFermentedJars` method which loops over the `fermentedJars` array and also has an external...

Vulnerable code:
if (party.assetChainId != getChainId() && address(honeyJarPortal) != address(0) && address(this).balance != 0) {
    uint256 sendAmount = address(this).balance / party.checkpoints.length;
    honeyJarPortal.sendFermentedJars{value: sendAmount}(
        address(this), party.assetChainId, party.bundleId, fermentedJars
    );
}
MEDIUM severity: Two people might win the same prize. **Impact:**
High, as people might not get their prizes

**Likelihood:**
Low, as it requires same `prizeIndex` wins

**Description**

The current way in `NFTLootbox` to decide if a player wins and what it wins is the `getPrizeIndex` method, which has the following implementation:

This shows that...

Vulnerable code:
uint256[] storage _probabilities = lootboxes[_lootboxId].probabilities;
uint256 sum;

// Calculate the cumulative sum of probabilities and find the winning prize
for (uint256 i; i < _probabilities.length; ++i) {
    sum += _probabilities[i];
    if (_randomNumber <= sum) {
        return i;
    }
}

// If no prize is won, return a missing prize index (100001)
return MAX_PROBABILITY + 1;
MEDIUM severity: Some vesting recipients temporarily won't be able to claim. **Impact:**
Medium, as funds will be locked for 30 days

**Likelihood:**
Medium, because it will only happen when the cliff is < 30 day

**Description**

The `SLICE_PERIOD` constant in `Vesting` is set to 30 days. Due to the following math in `_computeReleasableAmount`

If `timeFromStart` is less...

Vulnerable code:
uint256 vestedSeconds = (timeFromStart / SLICE_PERIOD) * SLICE_PERIOD;
LOW severity: Division before multiplication can lead to rounding errors. In `DutchAuction::priceFunction` there is division before multiplication here:

This can lead to rounding errors but without a serious impact to the application. Still you should always do multiplication before division in Solidity.

Vulnerable code:
uint256 priceDeclinePerSecond =
            (auctionDetails.startPrice - auctionDetails.minimumPrice) / auctionDuration;

return auctionDetails.startPrice - (timeSinceAuctionStart *
priceDeclinePerSecond);