Farseen0/scar-pairs huggingface dataset:
LOW severity: Not maximum amount of issuances enforce. **Description:** The `createIssuanceInformation` function in `ComplianceServiceRegulated.sol` allows unlimited issuance records to be created per investor without any maximum limit. Unlike the lock system which has a `MAX_LOCKS_PER_INVESTOR = 30` constant to prevent run out of gas, the issuance...

Vulnerable code:
function createIssuanceInformation(
        string memory _investor,
        uint256 _shares,
        uint256 _issuanceTime
    ) internal returns (bool) {
        uint256 issuancesCount = issuancesCounters[_investor];//@audit-ok (low) there is not max limit for the issuancesCounters?

        issuancesValues[_investor][issuancesCount] = _shares;
        issuancesTimestamps[_investor][issuancesCount] = _issuanceTime;
        issuancesCounters[_investor] = issuancesCount + 1;

        return true;
    }
LOW severity: Platform Wallet not exception for Maximum Holdings Per Investor Limits. **Description:** The `preIssuanceCheck` function in `ComplianceServiceRegulated.sol` exhibits inconsistent behavior regarding platform wallet exemptions for investor holdings limits. While platform wallets are properly exempted from the minimum holdings per investor check, they are not exempted...

Vulnerable code:
if (
    !_args.isPlatformWalletTo &&
    toInvestorBalance + _args.value < IDSComplianceConfigurationService(_services[COMPLIANCE_CONFIGURATION_SERVICE]).getMinimumHoldingsPerInvestor()
) {
    return (51, AMOUNT_OF_TOKENS_UNDER_MIN);
}

if (
            isMaximumHoldingsPerInvestorOk(
                IDSComplianceConfigurationService(_services[COMPLIANCE_CONFIGURATION_SERVICE]).getMaximumHoldingsPerInvestor(),
                toInvestorBalance, _args.value)
        ) {
            return (52, AMOUNT_OF_TOKENS_ABOVE_MAX);
        }

if (
+    !_args.isPlatformWalletTo &&
    isMaximumHoldingsPerInvestorOk(
        complianceConfigurationService.getMaximumHoldingsPerInvestor(),
        balanceOfInvestorTo,
        _value)
) {
    return (52, AMOUNT_OF_TOKENS_ABOVE_MAX);
}
LOW severity: Investor can prevent themselves from being removed by making `removeInvestor` revert. **Description:** The `removeInvestor` function in `RegistryService.sol` contains a flaw that allows any investor to permanently prevent their removal from the system. The function requires that `investors[_id].walletCount == 0` before allowing investor removal, but investors can add unlimited...

Vulnerable code:
function removeInvestor(string calldata _id) public override onlyExchangeOrAbove investorExists(_id) returns (bool) {
        require(getTrustService().getRole(msg.sender) != EXCHANGE || investors[_id].creator == msg.sender, "Insufficient permissions");
        require(investors[_id].walletCount == 0, "Investor has wallets"); <----------

        for (uint8 index = 0; index < 16; index++) {
            delete attributes[_id][index];
        }

        delete investors[_id];

        emit DSRegistryServiceInvestorRemoved(_id, msg.sender);

        return true;
    }
LOW severity: Resolve inconsistency between `DSToken::checkWalletsForList` and `RegistryService::removeWallet`. **Description:** `DSToken::checkWalletsForList` only removes wallets if their balance is zero:

But `RegistryService::removeWallet` allows removing wallets with positive balances:

**Impact:** Wallets with positive balances can be removed via `RegistryService::removeWallet` which prevents token...

Vulnerable code:
function checkWalletsForList(address _from, address _to) private {
    if (super.balanceOf(_from) == 0) {
        removeWalletFromList(_from);
    }

function removeWallet(address _address, string memory _id) public override onlyExchangeOrAbove walletExists(_address) walletBelongsToInvestor(_address, _id) returns (bool) {
    require(getTrustService().getRole(msg.sender) != EXCHANGE || investorsWallets[_address].creator == msg.sender, "Insufficient permissions");

    delete investorsWallets[_address];
    investors[_id].walletCount--;

    emit DSRegistryServiceWalletRemoved(_address, _id, msg.sender);

    return true;
}
HIGH severity: Prefer explicit unsigned integer sizes. **Description:** Prefer explicit unsigned integer sizes:

**Securitize:** Fixed in commit [26c5bb0](https://github.com/securitize-io/dstoken/commit/26c5bb05676233ca0c5caa1d9b02fd98360ad4e7).

**Cyfrin:** Verified.

Vulnerable code:
trust/TrustService.sol
218:        for (uint i = 0; i < _addresses.length; i++) {

compliance/ComplianceConfigurationService.sol
34:        for (uint i = 0; i < _countries.length; i++) {

compliance/WalletManager.sol
75:        for (uint i = 0; i < _wallets.length; i++) {
97:        for (uint i = 0; i < _wallets.length; i++) {
LOW severity: `TransactionRelayer` and `SecuritizeSwap` should use `CommonUtils::encodeString`. **Description:** `TransactionRelayer::toBytes32` duplicates functionality already available in `CommonUtils::encodeString`; remove the duplicated code and use `CommonUtils::encodeString` instead.

Also this line should use `CommonUtils::encodeString` instead of re-implementing it...

Vulnerable code:
L178:                        keccak256(abi.encodePacked(senderInvestor)),

191:                keccak256(abi.encodePacked(_senderInvestorId))
HIGH severity: More efficient way of checking for empty string in `CommonUtils::isEmptyString`. **Description:** More efficient way of checking for empty string in `CommonUtils::isEmptyString`:

**Securitize:** Fixed in commit [22b117a](https://github.com/securitize-io/dstoken/commit/22b117a3514c04b766aa7be6c855683865549e82).

**Cyfrin:** Verified.

Vulnerable code:
function isEmptyString(string memory _str) internal pure returns (bool) {
    return bytes(_str).length == 0;
}
LOW severity: More efficient way of comparing two strings for equality in `CommonUtils::isEqualString`. **Description:** More efficient way of comparing two strings for equality in `CommonUtils::isEqualString` [from](https://github.com/Vectorized/solady/blob/main/src/utils/g/LibString.sol#L858-L863) Solady:

**Securitize:** Acknowledged.

Vulnerable code:
function isEqualString(string memory a, string memory b) internal pure returns (bool result) {
      /// @solidity memory-safe-assembly
      assembly {
          result := eq(keccak256(add(a, 0x20), mload(a)), keccak256(add(b, 0x20), mload(b)))
      }
  }
LOW severity: Total supply and balance invariants can be broken by burning the same token id multiple times within a given batch operation. **Description:** The `_burnBatch(uint256[])` function loops over all provided `tokenIds` and updates the relevant state; however, the ownership/approval logic is executed for all tokens in `_burnBatch(address,uint256[],bool)` before any state updates have occurred. As such, this fails to...

Vulnerable code:
function totalSupply() public view returns (uint256) {
    unchecked {
        return _mintCounter - _burnCounter;
    }
}

unchecked {
    // Cannot overflow, as that would require more tokens to be burned/transferred
    // out than the owner initially received through minting and transferring in.
    _balances[owner] -= 1;
}

it("burning non-existent tokens", async function () {
  const { vfTokenC, owner } = await connection.networkHelpers.loadFixture(
    fixtureWithMintedTokensForBurning
  );

  await vfTokenC.connect(owner).toggleBurnActive();

  const balanceBefore = await vfTokenC.balanceOf(owner.address);
  const deadBefore = await vfTokenC.balanceOf(DEAD_ADDRESS);
  const totalSupplyBefore = await vfTokenC.totalSupply();

  await expect(vfTokenC.connect(owner).burnBatchAdmin([102, 102, 102, 102]))
    .to.emit(vfTokenC, "Transfer")
    .withArgs(owner.address, DEAD_ADDRESS, 102);

  const balanceAfter = await vfTokenC.balanceOf(owner.address);
  const deadAfter = await vfTokenC.balanceOf(DEAD_ADDRESS);
  const totalSupplyAfter = await vfTokenC.totalSupply();

  console.log("Balance before:", balanceBefore.toString());
  console.log("Balance after:", balanceAfter.toString());

  console.log("Dead before:", deadBefore.toString());
  console.log("Dead after:", deadAfter.toString());

  console.log("Total Supply before:", totalSupplyBefore.toString());
  console.log("Total Supply after:", totalSupplyAfter.toString());

  // Tokens should now be owned by DEAD_ADDRESS
  expect(await vfTokenC.ownerOf(102)).to.equal(DEAD_ADDRESS);

  // Owner balance should have decreased by one
  expect(balanceAfter).to.equal(balanceBefore - 1n);

  // Total supply should have only decreased by one (violated)
  expect(totalSupplyAfter).to.equal(totalSupplyBefore - 1n);

  // DEAD_ADDRESS balance should have increased by one (violated)
  expect(deadAfter).to.equal(deadBefore + 1n);
});
LOW severity: Inconsistent state updates when tokens are burned and/or transferred directly to the `DEAD_ADDRESS`. **Description:** The following state updates are performed when batch burning tokens:

Note how while the `_balances[owner]` state is decremented, that of `DEAD_ADDRESS` is not incremented despite being granted ownership.

Now consider the direct transfer of a token to the `DEAD_ADDRESS`. While...

Vulnerable code:
function _burnBatch(uint256[] calldata tokenIds) internal virtual {
        for (uint256 i; i < tokenIds.length; i++) {
            uint256 tokenId = tokenIds[i];
            address owner = ownerOf(tokenId);

            _beforeTokenTransfer(owner, DEAD_ADDRESS, tokenId, 1);

            // Clear approvals
@>          delete _tokenApprovals[tokenId];

            unchecked {
                // Cannot overflow, as that would require more tokens to be burned/transferred
                // out than the owner initially received through minting and transferring in.
@>              _balances[owner] -= 1;
            }

@>          _owners[tokenId] = DEAD_ADDRESS;

            emit Transfer(owner, DEAD_ADDRESS, tokenId);

            _afterTokenTransfer(owner, DEAD_ADDRESS, tokenId, 1);
        }

        unchecked {
@>          _burnCounter += tokenIds.length;
        }
    }

function _transfer(
        address from,
        address to,
        uint256 tokenId
    ) internal virtual {
        if (to == address(0)) {
            revert ERC721VFTransferToTheZeroAddress();
        }

        if (ownerOf(tokenId) != from) {
            revert ERC721VFTransferFromIncorrectOwner(from, tokenId);
        }

        _beforeTokenTransfer(from, to, tokenId, 1);

        // Clear approvals from the previous owner
        delete _tokenApprovals[tokenId];

        unchecked {
            // `_balances[from]` cannot overflow for the same reason as described in `_burn`:
            // `from`'s balance is the number of token held, which is at least one before the current
            // transfer.
            // `_balances[to]` could overflow in the conditions described in `_mint`. That would require
            // all 2**256 token ids to be minted, which in practice is impossible.
            _balances[from] -= 1;
@>          _balances[to] += 1;
        }
@>      _owners[tokenId] = to;

        emit Transfer(from, to, tokenId);

        _afterTokenTransfer(from, to, tokenId, 1);
    }
HIGH severity: Cancelling redeem requests permanently blocks the withdrawal queue. **Description:** `AccountableWithdrawalQueue` can deadlock at the head if the current head entry (`_queue.nextRequestId`) is fully removed (e.g., by a cancel that zeroes `shares` and clears `controller`) without advancing `nextRequestId`.

In...

Vulnerable code:
(uint256 shares_, uint256 assets_, bool processed_) =
    _processRequest(request_, liquidity, maxShares_, precision_);

if (shares_ == 0) break;

if (request.controller == address(0)) return (0, 0, true);

/// @dev Deletes a withdrawal request and its controller from the queue
function _delete(address controller, uint128 requestId) private {
    delete _queue.requests[requestId];
    delete _requestIds[controller];
}
HIGH severity: `AccountableAsyncRedeemVault::fulfillCancelRedeemRequest` can de-sync request data causing permanent DOS for queue processing. **Description:** `fulfillCancelRedeemRequest()` function first finalises the cancellation of the redeeem request with input `requestID`, and then calls `_reduce()` to update the request state and `totalQueuedShares`.

The problem here is that it is using current value of...

Vulnerable code:
function fulfillCancelRedeemRequest(address controller) public onlyOperatorOrStrategy {
        _fulfillCancelRedeemRequest(_requestIds[controller], controller);
        _reduce(controller, _vaultStates[controller].pendingRedeemRequest);
    }

function _fulfillRedeemRequest(uint128 requestId, address controller, uint256 shares, uint256 price)
        internal
        override
    {
        VaultState storage state = _vaultStates[controller];
        if (state.pendingRedeemRequest == 0) revert NoRedeemRequest();
        if (state.pendingRedeemRequest < shares) revert InsufficientAmount();
        if (state.pendingCancelRedeemRequest) revert RedeemRequestWasCancelled();

function fulfillCancelRedeemRequest(address controller) public onlyOperatorOrStrategy {

+++        uint256 pendingShares = state.pendingRedeemRequest;
               _fulfillCancelRedeemRequest(_requestIds[controller], controller);
---          _reduce(controller, _vaultStates[controller].pendingRedeemRequest);
+++        _reduce(controller, pendingShares);
    }
HIGH severity: Critical DOS in queue processing if async cancellations are allowed. **Description:** The `cancelRedeemRequest()` function can be used to DOS the queue processing (ie. `processUpToShares()` and `processUpToRequestID()` can be made to revert).

This is the attack path :
- `cancelRedeemRequest()` marks `state.pendingCancelRedeemRequest = true`;
- Assume that this...

Vulnerable code:
function cancelRedeemRequest(uint256 requestId, address controller) public onlyAuth {
        _checkController(controller);
        VaultState storage state = _vaultStates[controller];
        if (state.pendingRedeemRequest == 0) revert NoPendingRedeemRequest();
        if (state.pendingCancelRedeemRequest) revert CancelRedeemRequestPending();

        state.pendingCancelRedeemRequest = true;

        bool canCancel = strategy.onCancelRedeemRequest(address(this), controller); // @audit strategy can choose to return false here, thus mandating async cancellations.
        if (canCancel) {
            uint256 pendingShares = state.pendingRedeemRequest;

            _fulfillCancelRedeemRequest(uint128(requestId), controller);
            _reduce(controller, pendingShares);
        }
        emit CancelRedeemRequest(controller, requestId, msg.sender);
    }

function _fulfillRedeemRequest(uint128 requestId, address controller, uint256 shares, uint256 price)
        internal
        override
    {
        VaultState storage state = _vaultStates[controller];
        if (state.pendingRedeemRequest == 0) revert NoRedeemRequest();
        if (state.pendingRedeemRequest < shares) revert InsufficientAmount();
        if (state.pendingCancelRedeemRequest) revert RedeemRequestWasCancelled();  // @audit
HIGH severity: Partial redemptions can be used to steal assets. **Description:** The request state is not handled properly when redeem requests are filled partially, leading to an inflated redemption price for the remaining part of the request.

- When a new redemption is pushed onto an existing requestID, then the average redemption price is calculated using...

Vulnerable code:
} else { // if controller had an existing active requestID
            requestId = requestId_;

            WithdrawalRequest storage request = _queue.requests[requestId_];

            request.shares += shares;

            if (processingMode == ProcessingMode.RequestPrice) {
                request.totalValue += shares.mulDiv(sharePrice, _precision);
                request.sharePrice = request.totalValue.mulDiv(_precision, request.shares); // the average sharePrice is being calculated here.
            } // the whole request will have a single price, averaged recursively as new redeem requests come up.

        totalQueuedShares += shares;
    }

function _reduce(address controller, uint256 shares) internal returns (uint256 remainingShares) {
        uint128 requestId = _requestIds[controller];
        if (requestId == 0) revert NoQueueRequest();

        uint256 currentShares = _queue.requests[requestId].shares;
        if (shares > currentShares || currentShares == 0) revert InsufficientShares();

        remainingShares = currentShares - shares;
        totalQueuedShares -= shares;

        if (remainingShares == 0) {
            _delete(controller, requestId);
        } else {
            _queue.requests[requestId].shares = remainingShares;
        } // @audit the totalValue is not updated here.
    }
HIGH severity: `AccountableAsyncRedeemVault::fulfillRedeemRequest` ignores processingMode and directly uses currentPrice for finalizing a redeem request. **Description:** When a redeem request is placed using `requestRedeem` function, it pushes a new request struct into the withdrawal queue. If the processingMode of the vault is configured to be `== RequestPrice`, the current `sharePrice` at that time is stored as the "request.sharePrice" for later.....

Vulnerable code:
function fulfillRedeemRequest(address controller, uint256 shares) public onlyOperatorOrStrategy {
        _fulfillRedeemRequest(_requestIds[controller], controller, shares, sharePrice());
        _reduce(controller, shares);
    }

function fulfillRedeemRequest(address controller, uint256 shares) public onlyOperatorOrStrategy {
+++         uint256 price;
+++         if (processingMode == ProcessingMode.CurrentPrice)
+++              price = sharePrice();
+++       }
+++         else {
+++              uint128 requestId = _requestIds[controller];
+++              price = _queue.requests[requestId].sharePrice;
+++      }

               _fulfillRedeemRequest(_requestIds[controller], controller, shares, price);
               _reduce(controller, shares);
           }
HIGH severity: `AccountableOpenTerm` loan interest cannot be repaid once principal hits zero. **Description:** In `AccountableOpenTerm`, interest accrues virtually via `_scaleFactor`, but there is no mechanism to pay/realize that interest. The only funding paths are `borrow()`, `supply()`, and `repay()`. Both `supply()` and `repay()` first service withdrawals and then reduce...

Vulnerable code:
function test_openTerm_repay_principal_only_setsRepaid_no_interest_paid() public {
    vm.warp(1739893670);

    // Setup borrower & terms
    vm.prank(manager);
    usdcLoan.setPendingBorrower(borrower);
    vm.prank(borrower);
    usdcLoan.acceptBorrowerRole();

    LoanTerms memory terms = LoanTerms({
        minDeposit: 0,
        minRedeem: 0,
        maxCapacity: USDC_AMOUNT,
        minCapacity: USDC_AMOUNT / 2,
        interestRate: 150_000,           // 15% APR so scale factor grows visibly
        interestInterval: 30 days,
        duration: 0,
        depositPeriod: 2 days,
        acceptGracePeriod: 0,
        lateInterestGracePeriod: 0,
        lateInterestPenalty: 0,
        withdrawalPeriod: 0
    });
    vm.prank(manager);
    usdcLoan.setTerms(terms);
    vm.prank(borrower);
    usdcLoan.acceptTerms();

    // Single LP deposits during deposit period → 1:1 shares at PRECISION
    vm.prank(alice);
    usdcVault.deposit(USDC_AMOUNT, alice, alice);
    assertEq(usdcVault.totalAssets(), USDC_AMOUNT, "vault funded");

    // Borrower draws full principal → vault drained
    vm.prank(borrower);
    usdcLoan.borrow(USDC_AMOUNT);
    assertEq(usdcVault.totalAssets(), 0, "all assets borrowed");
    assertEq(usdcLoan.loan().outstandingPrincipal, USDC_AMOUNT, "principal outstanding");

    // Time passes → interest accrues virtually (scale factor > PRECISION)
    vm.warp(block.timestamp + 180 days);
    uint256 sfBefore = usdcLoan.accrueInterest();
    assertGt(sfBefore, 1e36, "scale factor increased (virtual interest)");

    // Borrower repays EXACTLY principal (no extra for interest)
    usdc.mint(borrower, USDC_AMOUNT);
    vm.startPrank(borrower);
    usdc.approve(address(usdcVault), type(uint256).max);
    usdcLoan.repay(USDC_AMOUNT);
    vm.stopPrank();

    // Loan marked repaid even though totalAssets < totalShareValue at sfBefore
    assertEq(uint8(usdcLoan.loanState()), uint8(LoanState.Repaid), "loan flipped to Repaid");

    // After Repaid, share price uses assetShareRatio (actual assets), not the higher scale factor.
    // With one LP and totalAssets == totalSupply, ratio == PRECISION → no interest realized.
    uint256 spAfter = usdcLoan.sharePrice(address(usdcVault));
    assertEq(spAfter, 1e36, "share price fell back to assetShareRatio (no interest paid)");

    // Sanity: vault now only holds repaid principal
    assertEq(usdcVault.totalAssets(), USDC_AMOUNT, "vault holds only principal after repay");
    assertEq(usdcVault.totalSupply(), USDC_AMOUNT, "shares unchanged");

    // Now borrower cannot "pay the interest" anymore
    vm.prank(borrower);
    vm.expectRevert(); // blocked by _requireLoanOngoing()
    usdcLoan.supply(1e6);
}
HIGH severity: Complete bypass of transfer restrictions on vault share token is possible. **Description:** In `AccountableVault.sol` (which is inherited by the `AccountableAsyncRedeemVault`, we have certain transfer restrictions (KYC, if from address is subject to a throttle timestamp), applied in `_checkTransfer()` function.

These restrictions are applied on `transfer()`/...

Vulnerable code:
function claimCancelRedeemRequest(uint256 requestId, address receiver, address controller)
        public
        onlyAuth
        returns (uint256 shares)
    {
        _checkController(controller);
        VaultState storage state = _vaultStates[controller];
        shares = state.claimableCancelRedeemRequest;
        if (shares == 0) revert ZeroAmount();

        strategy.onClaimCancelRedeemRequest(address(this), controller);

        state.claimableCancelRedeemRequest = 0;

        _transfer(address(this), receiver, shares); // @audit bypasses all transfer restrictions.

        emit CancelRedeemClaim(receiver, controller, requestId, msg.sender, shares);
    }
MEDIUM severity: `transferWhitelist` checks are missing in `AccountableVault::_checkTransfer`. **Description:** AccountableVault.sol employs a "transferWhitelist" feature to help select addresses that should be allowed to transfer vault shares, overriding the other restrictions checked in `_checkTransfer()`.

Both `transfer()` and `transferFrom()` functions internally call...

Vulnerable code:
/// @notice Mapping of addresses that can override transfer restrictions
    mapping(address => bool) public transferWhitelist;

function _checkTransfer(uint256 amount, address from, address to) private {

+++      if(transferWhitelist[from] && transferWhitelist[to]) return;

        if (amount == 0) revert ZeroAmount();
        if (!transferableShares) revert SharesNotTransferable();
        if (!isVerified(to, msg.data)) revert Unauthorized();
        if (throttledTransfers[from] > block.timestamp) revert TransferCooldown();
    }
MEDIUM severity: `AccountableAsyncRedeemVault` allows deposits for non-whitelisted or non-KYCed addresses. **Description:** Almost all functions in `AccountableAsyncRedeemVault` use an `onlyAuth()` modifier to verify that the caller is KYC-ed or Whitelisted (according to the vault's own policy).

This logic can be seen in `isVerified()` function in AccessBase.sol

Here is the...

Vulnerable code:
modifier onlyAuth() {
        if (!isVerified(msg.sender, msg.data)) revert Unauthorized();
        _;
    }

function deposit(uint256 assets, address receiver, address controller) public onlyAuth returns (uint256 shares) {
        _checkController(controller);
        if (assets == 0) revert ZeroAmount();
        if (assets > maxDeposit(controller)) revert ExceedsMaxDeposit();

        uint256 price = strategy.onDeposit(address(this), assets, receiver, controller);
        shares = _convertToShares(assets, price, Math.Rounding.Floor);

        _mint(receiver, shares);
        _deposit(controller, assets);