Farseen0/scar-pairs huggingface dataset:
MEDIUM severity: Incorrect EIP-712 `TXTYPE_HASH` in `SecuritizeSwap`. **Description:** `SecuritizeSwap::TXTYPE_HASH` does not match the actual message structure being encoded for EIP-712 signature verification. The constant is defined as the hash of:

But the actual `abi.encode` call in `SecuritizeSwap::doExecuteByInvestor` encodes a different structure:

The type...

Vulnerable code:
"ExecutePreApprovedTransaction(string memory _senderInvestor, address _destination,address _executor,bytes _data, uint256[] memory _params)"

function doExecuteByInvestor(
        uint8 _securitizeHsmSigV,
        bytes32 _securitizeHsmSigR,
        bytes32 _securitizeHsmSigS,
        string memory _senderInvestorId,
        address _destination,
        bytes memory _data,
        address _executor,
        uint256[] memory _params
    ) internal {
        bytes32 txInputHash = keccak256(
            abi.encode(
                TXTYPE_HASH,
                _destination,
                _params[0], //value
                keccak256(_data),
                noncePerInvestor[_senderInvestorId],
                _executor,
                _params[1], //gasLimit
                keccak256(abi.encodePacked(_senderInvestorId))
            )
        );
        ...
    }

// keccak256("ExecutePreApprovedTransaction(address destination,uint256 value,bytes32 data,uint256 nonce,address executor,uint256 gasLimit,bytes32 investorIdHash)")
bytes32 constant TXTYPE_HASH = 0xf13da213cea16aa5bb997703966334f85e5aa4d2b25964a7191e3a7bc08b7690;
MEDIUM severity: Changing investor country to the same country inflates investor count erroneously triggering max investor errors. **Description:** Changing investor country to the same country inflates investor count.

**Impact:** Inflating the investor count will erroneously trigger `MAX_INVESTORS_IN_CATEGORY` error in `ComplianceServiceRegulated::completeTransferCheck`. This does not require any error on the admin's part...

Vulnerable code:
it('Changing to the same country inflates investor count', async function() {
      const [wallet] = await hre.ethers.getSigners();
      const { dsToken, registryService, complianceConfigurationService, complianceService } = await loadFixture(deployDSTokenRegulated);

      // Setup
      await complianceConfigurationService.setCountryCompliance(INVESTORS.Country.USA, INVESTORS.Compliance.US);

      await registerInvestor(INVESTORS.INVESTOR_ID.INVESTOR_ID_1, wallet, registryService);
      await registryService.setAttribute(INVESTORS.INVESTOR_ID.INVESTOR_ID_1, 2, 1, 0, ""); // Make accredited

      // Set initial country and issue tokens
      await registryService.setCountry(INVESTORS.INVESTOR_ID.INVESTOR_ID_1, INVESTORS.Country.USA);
      await dsToken.setCap(1000);
      await dsToken.issueTokens(wallet, 100);

      // Verify initial state: 1 US investor
      expect(await complianceService.getUSInvestorsCount()).to.equal(1);

      // Change country from USA to USA
      await registryService.setCountry(INVESTORS.INVESTOR_ID.INVESTOR_ID_1, INVESTORS.Country.USA);

      // bug: us investor count increased even though it is the same investor
      //      and their country hasn't actually changed
      expect(await complianceService.getUSInvestorsCount()).to.equal(2);
    });
MEDIUM severity: Historical token issuances become subject to new lock-up requirements when users change residency. **Description:** The `checkHoldUp` function and `getComplianceTransferableTokens` function contain a flaw in how lock periods are applied to token issuances. The system incorrectly applies a single, current lock period to all historical issuances instead of using the lock period that was...

Vulnerable code:
function checkHoldUp(
    address[] memory _services,
    address _from,
    uint256 _value,
    bool _isUSLockPeriod,      // ← Determined at TRANSFER time
    bool _isPlatformWalletFrom
) internal view returns (bool) {
    uint256 lockPeriod;
    if (_isUSLockPeriod) {
        lockPeriod = getUSLockPeriod();     // ← Current US lock period
    } else {
        lockPeriod = getNonUSLockPeriod();  // ← Current Non-US lock period
    }

    // ← Applies same lock period to ALL issuances!
    return complianceService.getComplianceTransferableTokens(_from, block.timestamp, uint64(lockPeriod)) < _value;
}

function getComplianceTransferableTokens(
        address _who,
        uint256 _time,
        uint64 _lockTime
    ) public view override returns (uint256) {
        ...

        for (uint256 i = 0; i < investorIssuancesCount; i++) {
            uint256 issuanceTimestamp = issuancesTimestamps[investor][i];
             // Uses same _lockTime for ALL issuances <-------------
            if (uint256(_lockTime) > _time || issuanceTimestamp > (_time - uint256(_lockTime))) {
                uint256 tokens = getRebasingProvider().convertSharesToTokens(issuancesValues[investor][i]);
                totalLockedTokens = totalLockedTokens + tokens;
            }
        }

        //there may be more locked tokens than actual tokens, so the minimum between the two
        uint256 transferable = balanceOfInvestor - Math.min(totalLockedTokens, balanceOfInvestor);

        return transferable;
    }

// Add new mapping to ComplianceServiceDataStore.sol
mapping(string => mapping(uint256 => uint256)) issuanceLockPeriods;

function createIssuanceInformation(
    string memory _investor,
    uint256 _shares,
    uint256 _issuanceTime
) internal returns (bool) {
    ...

    issuancesValues[_investor][issuancesCount] = _shares;
    issuancesTimestamps[_investor][issuancesCount] = _issuanceTime;
+  issuanceLockPeriods[_investor][issuancesCount] = lockPeriod; // <----------Store lock period
    issuancesCounters[_investor] = issuancesCount + 1;

    return true;
}
MEDIUM severity: No way to revert `setInvestorLiquidateOnly`. **Description:** The `setInvestorLiquidateOnly` function in `InvestorLockManagerBase.sol` contains a logic error that prevents the disabling of liquidate-only mode once it has been enabled. The function includes a require statement that checks if the investor is already in liquidate-only mode and...

Vulnerable code:
**Impact:** Once an investor is set to liquidate-only mode, there is no way to disable this state


**Recommended Mitigation:** Remove the require statement to allow toggling of the liquidate-only state:
LOW severity: Multiplication could overflow in `RebasingLibrary` for tokens with greater than 18 decimals. **Description:** `RebasingLibrary` contains special handling for tokens with greater than 18 decimals:

**Impact:** When using tokens with high decimal values, the multiplication here could overflow causing denial of service.

**Recommended Mitigation:** Use OpenZeppelin's...

Vulnerable code:
// convertTokensToShares
        } else {
            uint256 scale = 10**(_tokenDecimals - 18);
            return (_tokens * DECIMALS_FACTOR + (_rebasingMultiplier * scale) / 2) / (_rebasingMultiplier * scale);
        }

// convertSharesToTokens
        } else {
            uint256 scale = 10**(_tokenDecimals - 18);
            return (_shares * _rebasingMultiplier * scale + DECIMALS_FACTOR / 2) / DECIMALS_FACTOR;
        }

pragma solidity 0.8.22;

import {Math} from "@openzeppelin/contracts/utils/math/Math.sol";

library RebasingLibrary {
    uint256 private constant DECIMALS_FACTOR = 1e18;

    function convertTokensToShares(
        uint256 _tokens,
        uint256 _rebasingMultiplier,
        uint8 _tokenDecimals
    ) internal pure returns (uint256 shares) {
        require(_rebasingMultiplier > 0, "Invalid rebasing multiplier");

        if (_tokenDecimals == 18) {
            return Math.mulDiv(_tokens, DECIMALS_FACTOR, _rebasingMultiplier);
        } else if (_tokenDecimals < 18) {
            uint256 scale = 10**(18 - _tokenDecimals);
            // tokens * scale * DECIMALS_FACTOR / multiplier
            return Math.mulDiv(_tokens * scale, DECIMALS_FACTOR, _rebasingMultiplier);
        } else {
            uint256 scale = 10**(_tokenDecimals - 18);
            // tokens * DECIMALS_FACTOR / (multiplier * scale)
            return Math.mulDiv(_tokens, DECIMALS_FACTOR, _rebasingMultiplier * scale);
        }
    }

    function convertSharesToTokens(
        uint256 _shares,
        uint256 _rebasingMultiplier,
        uint8 _tokenDecimals
    ) internal pure returns (uint256 tokens) {
        require(_rebasingMultiplier > 0, "Invalid rebasing multiplier");

        if (_tokenDecimals == 18) {
            return Math.mulDiv(_shares, _rebasingMultiplier, DECIMALS_FACTOR);
        } else if (_tokenDecimals < 18) {
            uint256 scale = 10**(18 - _tokenDecimals);
            // (shares * multiplier / DECIMALS_FACTOR) / scale
            return Math.mulDiv(_shares, _rebasingMultiplier, DECIMALS_FACTOR * scale);
        } else {
            uint256 scale = 10**(_tokenDecimals - 18);
            // shares * multiplier * scale / DECIMALS_FACTOR
            return Math.mulDiv(_shares * scale, _rebasingMultiplier, DECIMALS_FACTOR);
        }
    }
}
LOW severity: Asymmetry enforcement between `TokenIssuer::registerInvestor`, `WalletRegistrar::registerWallet` and `SecuritizeSwap::_registerNewInvestor`. **Description:** In `TokenIssuer::registerInvestor`, if the user isn't already an investor they get registered and must have 3 specific attributes set:

But in `WalletRegistrar::registerWallet` and `SecuritizeSwap::_registerNewInvestor` if the user isn't already an investor, they get registered...

Vulnerable code:
if (!getRegistryService().isInvestor(_id)) {
    getRegistryService().registerInvestor(_id, _collisionHash);
    getRegistryService().setCountry(_id, _country);

    if (_attributeValues.length > 0) {
        require(_attributeValues.length == 3, "Wrong length of parameters");
        getRegistryService().setAttribute(_id, KYC_APPROVED, _attributeValues[0], _attributeExpirations[0], "");
        getRegistryService().setAttribute(_id, ACCREDITED, _attributeValues[1], _attributeExpirations[1], "");
        getRegistryService().setAttribute(_id, QUALIFIED, _attributeValues[2], _attributeExpirations[2], "");
    }

if (!registryService.isInvestor(_id)) {
    registryService.registerInvestor(_id, _collisionHash);
    registryService.setCountry(_id, _country);
}

for (uint256 i = 0; i < _wallets.length; i++) {
    if (registryService.isWallet(_wallets[i])) {
        require(CommonUtils.isEqualString(registryService.getInvestor(_wallets[i]), _id), "Wallet belongs to a different investor");
    } else {
        registryService.addWallet(_wallets[i], _id);
    }
}

for (uint256 i = 0; i < _attributeIds.length; i++) {
    registryService.setAttribute(_id, _attributeIds[i], _attributeValues[i], _attributeExpirations[i], "");
}
LOW severity: Protocol will leak value to users due to rounding in `SecuritizeSwap::buy`. **Description:** Protocols should always round against users in favor of the protocol. The `buy` function in the `SecuritizeSwap.sol` is rounding the stable coin need it in favor of the user:

With this rounding down, in theory a user could be minting free DSTokens. However, since the DSToken has...

Vulnerable code:
function buy(uint256 _dsTokenAmount, uint256 _maxStableCoinAmount) public override whenNotPaused {
        ...
        uint256 stableCoinAmount = calculateStableCoinAmount(_dsTokenAmount); <----
        dsToken.issueTokensCustom(msg.sender, _dsTokenAmount, block.timestamp, 0, "", 0);

        emit Buy(msg.sender, _dsTokenAmount, stableCoinAmount, navProvider.rate());
    }

  function calculateStableCoinAmount(uint256 _dsTokenAmount) public view returns (uint256) {
        return _dsTokenAmount * navProvider.rate() / (10 ** ERC20(address(dsToken)).decimals()); // rounding down
    }
LOW severity: `TrustService::changeEntityOwner` can overwrite existing `_newOwner` record, breaking 1-1 relationship between owners and addresses. **Description:** `TrustService::changeEntityOwner` can overwrite existing `_newOwner` record, breaking 1-1 relationship between owners and addresses.

**Proof of Concept:** Add PoC to `test/trust-service.test.ts`:

Run with: `npx hardhat test --grep "overwrite existing owner"`.

**Recommended...

Vulnerable code:
it('overwrite existing owner breaks 1-1 relationship', async function() {
      const [owner, firstOwner, secondOwner] = await hre.ethers.getSigners();
      const { trustService } = await loadFixture(deployDSTokenRegulated);

      // Setup: Create two entities with different owners
      await trustService.setRole(firstOwner, DSConstants.roles.ISSUER);
      await trustService.setRole(secondOwner, DSConstants.roles.ISSUER);

      const entity1 = "Entity1";
      const entity2 = "Entity2";

      const trustServiceFromFirst = await trustService.connect(firstOwner);
      await trustServiceFromFirst.addEntity(entity1, firstOwner);

      const trustServiceFromSecond = await trustService.connect(secondOwner);
      await trustServiceFromSecond.addEntity(entity2, secondOwner);

      // Verify initial state
      expect(await trustService.getEntityByOwner(firstOwner)).equal(entity1);
      expect(await trustService.getEntityByOwner(secondOwner)).equal(entity2);

      // Change entity1 owner from firstOwner to secondOwner
      await trustService.changeEntityOwner(entity1, firstOwner, secondOwner);

      // Bug: secondOwner now owns both entities in the forward mapping
      // but reverse mapping shows only entity1
      expect(await trustService.getEntityByOwner(secondOwner)).equal(entity1);
      // entity2 is now orphaned - no way to find its owner through getEntityByOwner

      // firstOwner has no entity in reverse mapping
      expect(await trustService.getEntityByOwner(firstOwner)).equal("");
    });
LOW severity: `TrustService::removeRole` doesn't delete already owned entities so address which lost role can still manage existing entities. **Description:** `TrustService::removeRole` doesn't delete entities so address which lost role can still manage existing entities.

**Proof of Concept:** Add PoC to `test/trust-service.test.ts`:

**Recommended Mitigation:** When an address loses its role, delete entities it previously owned by...

Vulnerable code:
it('Should demonstrate orphaned entity relationships after role removal', async function() {
      const [owner, entityOwner, operator, resource] = await hre.ethers.getSigners();
      const { trustService } = await loadFixture(deployDSTokenRegulated);

      // Step 1: Give entityOwner ISSUER role
      await trustService.setRole(entityOwner, DSConstants.roles.ISSUER);
      expect(await trustService.getRole(entityOwner)).equal(DSConstants.roles.ISSUER);

      // Step 2: Create an entity owned by entityOwner
      const entityName = "TestEntity";
      const trustServiceFromEntityOwner = await trustService.connect(entityOwner);
      await trustServiceFromEntityOwner.addEntity(entityName, entityOwner);

      // Verify entity ownership
      expect(await trustService.getEntityByOwner(entityOwner)).equal(entityName);

      // Step 3: Add operator and resource to the entity
      await trustServiceFromEntityOwner.addOperator(entityName, operator);
      await trustServiceFromEntityOwner.addResource(entityName, resource);

      // Verify operator and resource are linked to entity
      expect(await trustService.getEntityByOperator(operator)).equal(entityName);
      expect(await trustService.getEntityByResource(resource)).equal(entityName);

      // Step 4: Remove entityOwner's ISSUER role
      await trustService.removeRole(entityOwner);
      expect(await trustService.getRole(entityOwner)).equal(DSConstants.roles.NONE);

      // Step 5: Demonstrate the bug - entity relationships still exist
      // These should ideally be cleaned up but they're not:
      expect(await trustService.getEntityByOwner(entityOwner)).equal(entityName);  // Still owns entity!
      expect(await trustService.getEntityByOperator(operator)).equal(entityName);   // Still linked!
      expect(await trustService.getEntityByResource(resource)).equal(entityName);  // Still linked!

      // Step 6: Show the security issue - entityOwner can still manage the entity
      // even without any role
      await expect(
        trustServiceFromEntityOwner.addOperator(entityName, hre.ethers.Wallet.createRandom())
      ).to.not.be.reverted;  // This should fail but doesn't!

      // The onlyEntityOwnerOrAbove modifier still passes because:
      // - entityOwner has NONE role (not MASTER/ISSUER)
      // - But ownersEntities[entityOwner] still equals entityName
      // - So the check passes even though they shouldn't have access
    });
LOW severity: Stale Issuance Records After `Burn` and `Seize` Operations. **Description:** The `recordBurn()` and `recordSeize()` functions in `ComplianceServiceRegulated.sol` do not clean up issuance records when tokens are burned or seized. This causes stale issuance records to persist in the system, which can lead to incorrect lock calculations for newly issued...

Vulnerable code:
function recordBurn(address _who, uint256 _value) internal override returns (bool) {

        if (compareInvestorBalance(_who, _value, _value)) {
            adjustTotalInvestorsCounts(_who, CommonUtils.IncDec.Decrease);
        }
        return true;
    }
HIGH severity: `SecuritizeSwap::executeStableCoinTransfer` should use `SafeER` approval and transfer functions instead of standard `IER` functions. **Description:** In `SecuritizeSwap::executeStableCoinTransfer` use [`SafeERC20::forceApprove`](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/utils/SafeERC20.sol#L101-L108) and...

Vulnerable code:
function executeStableCoinTransfer(address from, uint256 value) private {
        if (bridgeChainId != 0 && address(USDCBridge) != address(0)) {
-           stableCoinToken.transferFrom(from, address(this), value);
-           stableCoinToken.approve(address(USDCBridge), value);
+           SafeERC20.safeTransferFrom(stableCoinToken, from, address(this), value);
+           SafeERC20.forceApprove(stableCoinToken, address(USDCBridge), value);
            USDCBridge.sendUSDCCrossChainDeposit(bridgeChainId, issuerWallet, value);
        } else {
-           stableCoinToken.transferFrom(from, issuerWallet, value);
+           SafeERC20.safeTransferFrom(stableCoinToken, from, issuerWallet, value);
        }
    }
LOW severity: `MulticallProxy::_callTarget` doesn't check if `target` has code. **Description:** `IssuerMulticall::multicall` allows permissioned users to perform multiple calls against arbitrary targets, calling `MulticallProxy::_callTarget` for each target.

`MulticallProxy::_callTarget` doesn't check whether each `target` has code:

**Impact:** When using `call` on an...

Vulnerable code:
function _callTarget(address target, bytes memory data, uint256 i) internal returns (bytes memory) {
    // @audit returns true if `target` has no code
    (bool success, bytes memory returndata) = target.call(data);
    if (!success) {
        if (returndata.length > 0) {
            // Assumes revert reason is encoded as string
            revert MulticallFailed(i, _getRevertReason(returndata));
        } else {
            revert MulticallFailed(i, "Call failed without revert reason");
        }
    }
    return returndata;
}

function _callTarget(address target, bytes memory data, uint256 i) internal returns (bytes memory) {
+        require(target.code.length > 0, "Target is not a contract");
LOW severity: `ComplianceServiceRegulated::preIssuanceCheck` allows issuance to non-accredited investors when `forceAccredited` or `forceAccreditedUS` is set, and allows issuance below regional minimum thresholds, violating compliance requirements. **Description:** `ComplianceServiceRegulated::completeTransferCheck` has several checks that `preIssuanceCheck` is missing:

1) Force Accredited

`completeTransferCheck` verifies whether force accredited is enabled and if so only allow transfers to accredited investors:

But `preIssuanceCheck`...

Vulnerable code:
bool isAccreditedTo = isAccredited(_services, _args.to);
if (
    IDSComplianceConfigurationService(_services[COMPLIANCE_CONFIGURATION_SERVICE]).getForceAccredited() && !isAccreditedTo
) {
    return (61, ONLY_ACCREDITED);
}

} else if (toRegion == US) {
    if (
        IDSComplianceConfigurationService(_services[COMPLIANCE_CONFIGURATION_SERVICE]).getForceAccreditedUS() &&
        !isAccreditedTo
    ) {
        return (62, ONLY_US_ACCREDITED);
    }

if (toInvestorBalance + _value < IDSComplianceConfigurationService(_services[COMPLIANCE_CONFIGURATION_SERVICE]).getMinUSTokens()) {
    return (51, AMOUNT_OF_TOKENS_UNDER_MIN);
}

if (
            !walletManager.isPlatformWallet(_to) &&
        balanceOfInvestorTo + _value < complianceConfigurationService.getMinimumHoldingsPerInvestor()
        ) {
            return (51, AMOUNT_OF_TOKENS_UNDER_MIN);
        }
LOW severity: Burn and seize functions can be DoS when investor has several wallets that they control. **Description:** The `burn` function in `TokenLibrary.sol` checks `walletsBalances[_who]` (individual wallet balance, see the arrow above) instead of `investorsBalances[investorId]` (total investor balance across all wallets). This allows investors with multiple registered wallets to make their...

Vulnerable code:
function burn(
        TokenData storage _tokenData,
        address[] memory _services,
        address _who,
        uint256 _value,
        ISecuritizeRebasingProvider _rebasingProvider
    ) public returns (uint256) {
        uint256 sharesToBurn = _rebasingProvider.convertTokensToShares(_value);

        require(sharesToBurn <= _tokenData.walletsBalances[_who], "Not enough balance"); <---------

        IDSComplianceService(_services[COMPLIANCE_SERVICE]).validateBurn(_who, _value);

        _tokenData.walletsBalances[_who] -= sharesToBurn;
        updateInvestorBalance(
            _tokenData,
            IDSRegistryService(_services[REGISTRY_SERVICE]),
            _who,
            sharesToBurn,
            CommonUtils.IncDec.Decrease
        );

        _tokenData.totalSupply -= sharesToBurn;
        return sharesToBurn;
    }

describe('Burn DoS Vulnerability POC', function() {
        it('Should demonstrate that burn can be DoS by transferring between investor wallets', async function() {
          const [investor, wallet2, wallet3] = await hre.ethers.getSigners();
          const { dsToken, registryService } = await loadFixture(deployDSTokenRegulatedWithRebasingAndEighteenDecimal);

          // Register investor with multiple wallets
          await registerInvestor(INVESTORS.INVESTOR_ID.INVESTOR_ID_1, investor.address, registryService);
          await registryService.addWallet(wallet2.address, INVESTORS.INVESTOR_ID.INVESTOR_ID_1);
          await registryService.addWallet(wallet3.address, INVESTORS.INVESTOR_ID.INVESTOR_ID_1);

          // Issue tokens to investor
          await dsToken.issueTokens(investor.address, 1000);

          // Transfer tokens between investor's own wallets
          await dsToken.connect(investor).transfer(wallet2.address, 1000);

          // Now investor has 0 balance in original wallet but 1000 total
          expect(await dsToken.balanceOf(investor.address)).to.equal(0);
          expect(await dsToken.balanceOfInvestor(INVESTORS.INVESTOR_ID.INVESTOR_ID_1)).to.equal(1000);

          // VULNERABILITY: Burn fails because wallet balance is 0, even though investor has tokens
          await expect(dsToken.burn(investor.address, 100, 'DoS test'))
            .to.be.revertedWith('Not enough balance');
        });
      });