Farseen0/scar-pairs huggingface dataset:
HIGH severity: Lack of Governance action to update registered emitters. **Description:** The Wormhole CCTP integration contract currently exposes a function [`Governance::registerEmitterAndDomain`](https://github.com/wormhole-foundation/wormhole-circle-integration/blob/f7df33b159a71b163b8b5c7e7381c0d8f193da99/evm/src/contracts/CircleIntegration/Governance.sol#L48-L84).....

Vulnerable code:
function registerEmitterAndDomain(bytes memory encodedVaa) public {
    /* snip: parsing of Governance VAA payload */

    // Set the registeredEmitters state variable.
    registeredEmitters[foreignChain] = foreignAddress;

    // update the chainId to domain (and domain to chainId) mappings
    getChainToDomain()[foreignChain] = cctpDomain;
    getDomainToChain()[cctpDomain] = foreignChain;
}
LOW severity: Temporary denial-of-service when in-flight messages are not executed before a deprecated Wormhole Guardian set expires. **Description:** Wormhole exposes a governance action in [`Governance::submitNewGuardianSet`](https://github.com/wormhole-foundation/wormhole/blob/eee4641f55954d2d0db47831688a2e97eb20f7ee/ethereum/contracts/Governance.sol#L76-L112) to update the Guardian set via Governance VAA.

When this function.....

Vulnerable code:
function submitNewGuardianSet(bytes memory _vm) public {
    ...

    // Trigger a time-based expiry of current guardianSet
    expireGuardianSet(getCurrentGuardianSetIndex());

    // Add the new guardianSet to guardianSets
    storeGuardianSet(upgrade.newGuardianSet, upgrade.newGuardianSetIndex);

    // Makes the new guardianSet effective
    updateGuardianSetIndex(upgrade.newGuardianSetIndex);
}

function expireGuardianSet(uint32 index) internal {
    _state.guardianSets[index].expirationTime = uint32(block.timestamp) + 86400;
}

/// @dev Checks if VM guardian set index matches the current index (unless the current set is expired).
if(vm.guardianSetIndex != getCurrentGuardianSetIndex() && guardianSet.expirationTime < block.timestamp){
    return (false, "guardian set has expired");
}
LOW severity: Silent overflow in `TrimmedAmount::shift` could result in rate limiter being bypassed. **Description:** Within [`TrimmedAmount::trim`](https://github.com/wormhole-foundation/example-native-token-transfers/blob/f4e2277b358349dbfb8a654d19a925628d48a8af/evm/src/libraries/TrimmedAmount.sol#L136-L158), there is an explicit check that ensures the scaled amount does not exceed the maximum...

Vulnerable code:
// NOTE: amt after trimming must fit into uint64 (that's the point of
// trimming, as Solana only supports uint64 for token amts)
if (amountScaled > type(uint64).max) {
    revert AmountTooLarge(amt);
}

function shift(
    TrimmedAmount memory amount,
    uint8 toDecimals
) internal pure returns (TrimmedAmount memory) {
    uint8 actualToDecimals = minUint8(TRIMMED_DECIMALS, toDecimals);
    return TrimmedAmount(
        uint64(scale(amount.amount, amount.decimals, actualToDecimals)), actualToDecimals
    );
}
LOW severity: Disabled Transceivers cannot be re-enabled by calling `TransceiverRegistry::_setTransceiver` after 64 have been registered. **Description:** [`TransceiverRegistry::_setTransceiver`](https://github.com/wormhole-foundation/example-native-token-transfers/blob/f4e2277b358349dbfb8a654d19a925628d48a8af/evm/src/NttManager/TransceiverRegistry.sol#L112-L153) handles the registering of Transceivers, but note that they cannot be...

Vulnerable code:
function _setTransceiver(address transceiver) internal returns (uint8 index) {
    /* snip */
    if (transceiver == address(0)) {
        revert InvalidTransceiverZeroAddress();
    }

    if (_numTransceivers.registered >= MAX_TRANSCEIVERS) {
        revert TooManyTransceivers();
    }

    if (transceiverInfos[transceiver].registered) {
        transceiverInfos[transceiver].enabled = true;
    } else {
    /* snip */
}
MEDIUM severity: NTT Manager cannot be unpaused once paused. **Description:** `NttManagerState::pause` exposes pause functionality to be triggered by permissioned actors but has no corresponding unpause functionality. As such, once the NTT Manager is paused, it will not be possible to unpause without a contract upgrade.

**Impact:** The inability to unpause.....

Vulnerable code:
function pause() public onlyOwnerOrPauser {
    _pause();
}

+ function unpause() public onlyOwnerOrPauser {
+     _unpause();
+ }
LOW severity: Transceiver invariants and ownership synchronicity can be broken by unsafe Transceiver upgrades. **Description:** Transceivers are upgradeable contracts integral to the cross-chain message handling of NTT tokens. While `WormholeTransceiver` is a specific implementation of the `Transceiver` contract, NTT Managers can integrate with Transceivers of any custom...

Vulnerable code:
function _initialize() internal virtual override {
    // check if the owner is the deployer of this contract
    if (msg.sender != deployer) {
        revert UnexpectedDeployer(deployer, msg.sender);
    }

    __ReentrancyGuard_init();
    // owner of the transceiver is set to the owner of the nttManager
    __PausedOwnable_init(msg.sender, getNttManagerOwner());
}

/// @dev transfer the ownership of the transceiver to a new address
/// the nttManager should be able to update transceiver ownership.
function transferTransceiverOwnership(address newOwner) external onlyNttManager {
    _transferOwnership(newOwner);
}

/// @notice Transfer ownership of the Manager contract and all Endpoint contracts to a new owner.
function transferOwnership(address newOwner) public override onlyOwner {
    super.transferOwnership(newOwner);
    // loop through all the registered transceivers and set the new owner of each transceiver to the newOwner
    address[] storage _registeredTransceivers = _getRegisteredTransceiversStorage();
    _checkRegisteredTransceiversInvariants();

    for (uint256 i = 0; i < _registeredTransceivers.length; i++) {
        ITransceiver(_registeredTransceivers[i]).transferTransceiverOwnership(newOwner);
    }
}
HIGH severity: Inconsistent implementation of ERNamespaced Storage locations. **Description:** Regarding the implementation of Namespaced Storage locations, the EIP-7201 formula should be followed to strictly conform to the EIP specification, as is correctly done by the modified OpenZeppelin libraries in `libraries/external`; elsewhere, usage is incorrect.

Additionally,...

Vulnerable code:
keccak256(abi.encode(uint256(keccak256("example.location")) - 1)) & ~bytes32(uint256(0xff));
LOW severity: Asymmetry in Transceiver pausing capability. **Description:** Pausing functionality is exposed via `Transceiver::_pauseTransceiver`; however, there is no corresponding function that exposes unpausing functionality:

**Impact:** While not an immediate issue since the above function is not currently in use anywhere, this should be resolved to...

Vulnerable code:
/// @dev pause the transceiver.
function _pauseTransceiver() internal {
    _pause();
}

+ /// @dev unpause the transceiver.
+ function _unpauseTransceiver() internal {
+     _unpause();
+ }
LOW severity: Incorrect Transceiver payload prefix definition. **Description:** The `WH_TRANSCEIVER_PAYLOAD_PREFIX` constant in `WormholeTransceiverState.sol` contains invalid ASCII bytes and, as such, does not match what is written in the inline developer documentation:

The correct payload prefix is `0x99455748`, which is output when running the following...

Vulnerable code:
/// @dev Prefix for all TransceiverMessage payloads
///      This is 0x99'E''W''H'
/// @notice Magic string (constant value set by messaging provider) that idenfies the payload as an transceiver-emitted payload.
///         Note that this is not a security critical field. It's meant to be used by messaging providers to identify which messages are Transceiver-related.
bytes4 constant WH_TRANSCEIVER_PAYLOAD_PREFIX = 0x9945FF10;

+ bytes4 constant WH_TRANSCEIVER_PAYLOAD_PREFIX = 0x99455748;
LOW severity: `WormholeTransceiver` EVM chain IDs storage cannot be updated. In the unlikely but possible scenario in which a registered EVM-compatible chain diverges as the result of an upgrade, the existing implementation of [`WormholeTransceiverState::setIsWormholeEvmChain`](https://github.com/wormhole-foundation/example-native-token-transfers/blob/f4e2277b358349dbfb8a654...

Vulnerable code:
- function setIsWormholeEvmChain(uint16 chainId) external onlyOwner {
+ function setIsWormholeEvmChain(uint16 chainId, bool isEvm) external onlyOwner {
    if (chainId == 0) {
        revert InvalidWormholeChainIdZero();
    }
-     _getWormholeEvmChainIdsStorage()[chainId] = TRUE;
+     _getWormholeEvmChainIdsStorage()[chainId] = toWord(isEvm);

-     emit SetIsWormholeEvmChain(chainId);
+     emit SetIsWormholeEvmChain(chainId, isEvm);
}
LOW severity: Unchained initializers should be called in `PausableOwnable::__PausedOwnable_init` instead. While not an immediate issue in the current implementation, the direct use of initializer functions rather than their unchained equivalents should be avoided. These have been implemented and used in some places but not others, for example,...

Vulnerable code:
function __PausedOwnable_init(address initialPauser, address owner) internal onlyInitializing {
    __Paused_init(initialPauser);
    __Ownable_init(owner);
}
LOW severity: NatSpec documentation inconsistent with function/event signatures. Inline NatSpec documentation does not reflect the actual signatures in multiple cases:

1. Missing definition for the `recipientNttManagerAddress` in the `ITransceiver::sendMessage` [NatSpec](https://github.com/wormhole-foundation/example-native-token-transfers/blob/f4e2277b358349dbfb8a654d19a925628...

Vulnerable code:
/// @dev Send a message to another chain.
/// @param recipientChain The Wormhole chain ID of the recipient.
/// @param instruction An additional Instruction provided by the Transceiver to be
/// executed on the recipient chain.
/// @param nttManagerMessage A message to be sent to the nttManager on the recipient chain.
function sendMessage(
    uint16 recipientChain, //@note wormhole chain id
    TransceiverStructs.TransceiverInstruction memory instruction, //@note same as above
    bytes memory nttManagerMessage,
    bytes32 recipientNttManagerAddress
) external payable;

/// @notice Emitted when an outbound transfer is rate limited.
/// @dev Topic0
///      0x754d657d1363ee47d967b415652b739bfe96d5729ccf2f26625dcdbc147db68b.
/// @param sender The initial sender of the transfer.
/// @param amount The amount to be transferred.
/// @param currentCapacity The capacity left for transfers within the 24-hour window.
event OutboundTransferRateLimited(
    address indexed sender, uint64 sequence, uint256 amount, uint256 currentCapacity
);
LOW severity: Unused `PausableUpgradeable::CannotRenounceWhilePaused` error should be removed. [`PausableUpgradeable::CannotRenounceWhilePaused`](https://github.com/wormhole-foundation/example-native-token-transfers/blob/f4e2277b358349dbfb8a654d19a925628d48a8af/evm/src/libraries/PausableUpgradeable.sol#L50-L53) is a custom error defined as follows:

The above error and inline comments imply.....

Vulnerable code:
/**
 * @dev Cannot renounce the pauser capability when the contract is in the `PAUSED` state
 */
error CannotRenounceWhilePaused(address account);

/**
 * @dev Transfers the ability to pause to a new account (`newPauser`).
 */
function transferPauserCapability(address newPauser) public virtual onlyOwnerOrPauser {
    PauserStorage storage $ = _getPauserStorage();
    address oldPauser = $._pauser;
    $._pauser = newPauser;
    emit PauserTransferred(oldPauser, newPauser);
}
HIGH severity: Multiple `staticcall` success booleans are not checked. The success boolean returned when performing a `staticcall` on the given ERC20 token is not currently checked for any of the instances. It is understood this is a trust assumption in that legitimate implementations should always have `IERC20::decimals` implemented and should not revert when...

Vulnerable code:
function _getTokenBalanceOf(
    address tokenAddr,
    address accountAddr
) internal view returns (uint256) {
    (, bytes memory queriedBalance) =
        tokenAddr.staticcall(abi.encodeWithSelector(IERC20.balanceOf.selector, accountAddr));
    return abi.decode(queriedBalance, (uint256));
}
LOW severity: Canonical NTT chain ID should be fetched directly from Wormhole or mapped accordingly. Currently, the immutable `chainId` variable is assigned the value passed to the `NttManager` [constructor](https://github.com/wormhole-foundation/example-native-token-transfers/blob/f4e2277b358349dbfb8a654d19a925628d48a8af/evm/src/NttManager/NttManager.sol#L26-L32
) by the deployer:

However, no...

Vulnerable code:
constructor(
    address _token,
    Mode _mode,
    uint16 _chainId,
    uint64 _rateLimitDuration
) NttManagerState(_token, _mode, _chainId, _rateLimitDuration) {}
LOW severity: Unnecessary type casts of proxied storage slots from `bytes32` to `uint256`. There are multiple instances where proxied storage slots are cast from type `bytes32` to `uint256`; however, this is not necessary as the EVM uses 32-byte words, meaning that these types are interchangeable when considering the subsequent inline assembly assignment. One example can be found in...

Vulnerable code:
function _getConsumedGovernanceActionsStorage()
    private
    pure
    returns (mapping(bytes32 => bool) storage $)
{
    uint256 slot = uint256(CONSUMED_GOVERNANCE_ACTIONS_SLOT);
    assembly ("memory-safe") {
        $.slot := slot
    }
}
LOW severity: Unnecessary stack variable in `Governance::encodeGeneralPurposeGovernanceMessage`. [`Governance::encodeGeneralPurposeGovernanceMessage`](https://github.com/wormhole-foundation/example-native-token-transfers/blob/f4e2277b358349dbfb8a654d19a925628d48a8af/evm/src/wormhole/Governance.sol#L126-L144) currently assigns the stack variable `callDataLength`; however, this is not necessary.....

Vulnerable code:
function encodeGeneralPurposeGovernanceMessage(GeneralPurposeGovernanceMessage memory m)
    public
    pure
    returns (bytes memory encoded)
{
    if (m.callData.length > type(uint16).max) {
        revert PayloadTooLong(m.callData.length);
    }
    uint16 callDataLength = uint16(m.callData.length);
    return abi.encodePacked(
        MODULE,
        m.action,
        m.chain,
        m.governanceContract,
        m.governedContract,
        callDataLength,
        m.callData
    );
}
LOW severity: Optimize the `TrimmedAmount` struct as a bit-packed `uint72` user-defined type. The existing [`TrimmedAmount`](https://github.com/wormhole-foundation/example-native-token-transfers/blob/f4e2277b358349dbfb8a654d19a925628d48a8af/evm/src/libraries/TrimmedAmount.sol#L5-L8) abstraction has a [runtime gas overhead](https://soliditylang.org/blog/2021/09/27/user-defined-value-types/).....

Vulnerable code:
struct TrimmedAmount {
    uint64 amount;
    uint8 decimals;
}

struct RateLimitParams {
        TrimmedAmount limit;
        TrimmedAmount currentCapacity;
        uint64 lastTxTimestamp;
    }
LOW severity: Optimize invariant check iterations in `TransceiverRegistry::_checkTransceiversInvariants`. The first loop over all enabled transceivers in [`TransceiverRegistry::_checkTransceiversInvariants`](https://github.com/wormhole-foundation/example-native-token-transfers/blob/f4e2277b358349dbfb8a654d19a925628d48a8af/evm/src/NttManager/TransceiverRegistry.sol#L211-L234) could instead be included...

Vulnerable code:
function _checkTransceiversInvariants() internal view {
    _NumTransceivers storage _numTransceivers = _getNumTransceiversStorage();
    address[] storage _enabledTransceivers = _getEnabledTransceiversStorage();

    uint256 numTransceiversEnabled = _numTransceivers.enabled;
    assert(numTransceiversEnabled == _enabledTransceivers.length);

-   for (uint256 i = 0; i < numTransceiversEnabled; i++) {
-       _checkTransceiverInvariants(_enabledTransceivers[i]);
-   }
-
    // invariant: each transceiver is only enabled once
    for (uint256 i = 0; i < numTransceiversEnabled; i++) {
+       _checkTransceiverInvariants(_enabledTransceivers[i]);
        for (uint256 j = i + 1; j < numTransceiversEnabled; j++) {
            assert(_enabledTransceivers[i] != _enabledTransceivers[j]);
        }
    }

    // invariant: numRegisteredTransceivers <= MAX_TRANSCEIVERS
    assert(_numTransceivers.registered <= MAX_TRANSCEIVERS);
}
HIGH severity: `Goldilend.lock()` will always revert. **Severity:** High

**Description:** In `lock()`, it calls `_refreshiBGT()` before pulling `iBGT` from the user and will revert while calling `iBGTVault(ibgtVault).stake()`.

**Impact:** Users can't lock `iBGT` as `lock()` always reverts.

**Recommended Mitigation:** `_refreshiBGT()` should be...

Vulnerable code:
function lock(uint256 amount) external {
    uint256 mintAmount = _GiBGTMintAmount(amount);
    poolSize += amount;
    _refreshiBGT(amount); //@audit should call after depositing funds
    SafeTransferLib.safeTransferFrom(ibgt, msg.sender, address(this), amount);
    _mint(msg.sender, mintAmount);
    emit iBGTLock(msg.sender, amount);
  }
...
  function _refreshiBGT(uint256 ibgtAmount) internal {
    ERC20(ibgt).approve(ibgtVault, ibgtAmount);
    iBGTVault(ibgtVault).stake(ibgtAmount); //@audit will revert here
  }
HIGH severity: Wrong `PoolSize` increment in `Goldilend.repay()`. **Severity:** High

**Description:** When a user repays his loan using `repay()`, it increases `poolSize` with the repaid interest. During the increment, it uses the wrong amount.

It should use `interest` instead of `userLoan.interest` because the user repaid `interest` only.

**Impact:**...

Vulnerable code:
function repay(uint256 repayAmount, uint256 _userLoanId) external {
    Loan memory userLoan = loans[msg.sender][_userLoanId];
    if(userLoan.borrowedAmount < repayAmount) revert ExcessiveRepay();
    if(block.timestamp > userLoan.endDate) revert LoanExpired();
    uint256 interestLoanRatio = FixedPointMathLib.divWad(userLoan.interest, userLoan.borrowedAmount);
    uint256 interest = FixedPointMathLib.mulWadUp(repayAmount, interestLoanRatio);
    outstandingDebt -= repayAmount - interest > outstandingDebt ? outstandingDebt : repayAmount - interest;
    loans[msg.sender][_userLoanId].borrowedAmount -= repayAmount;
    loans[msg.sender][_userLoanId].interest -= interest;
    poolSize += userLoan.interest * (1000 - (multisigShare + apdaoShare)) / 1000; //@audit should use interest instead of userLoan.interest
...
  }
LOW severity: Users can extend an expired boost using invalidated NFTs.. **Severity:** High

**Description:** In `Goldilend.sol#L251`, a user can extend a boost with invalidated NFTs.
- The user has created a boost with a valid NFT.
- After that, the NFT was invalidated using `adjustBoosts()`.
- After the original boost is expired, the user can just call `boost()` with.....

Vulnerable code:
function _buildBoost(
    address[] calldata partnerNFTs,
    uint256[] calldata partnerNFTIds
  ) internal returns (Boost memory newUserBoost) {
    uint256 magnitude;
    Boost storage userBoost = boosts[msg.sender];
    if(userBoost.expiry == 0) {
...
    }
    else {
      address[] storage nfts = userBoost.partnerNFTs;
      uint256[] storage ids = userBoost.partnerNFTIds;
      magnitude = userBoost.boostMagnitude; //@audit use old magnitude without checking
      for (uint256 i = 0; i < partnerNFTs.length; i++) {
        magnitude += partnerNFTBoosts[partnerNFTs[i]];
        nfts.push(partnerNFTs[i]);
        ids.push(partnerNFTIds[i]);
      }
      newUserBoost = Boost({
        partnerNFTs: nfts,
        partnerNFTIds: ids,
        expiry: block.timestamp + boostLockDuration,
        boostMagnitude: magnitude
      });
    }
  }
HIGH severity: Team members can't unstake the initial allocation forever.. **Severity:** High

**Description:** When users call `unstake()`, it calculates the vested amount using `_vestingCheck()`.

But it returns 0 for team members and they can't unstake forever.
Furthermore, in `stake()`, it just prevents seed investors, not team members. So if team members have staked.....

Vulnerable code:
function _vestingCheck(address user, uint256 amount) internal view returns (uint256) {
    if(teamAllocations[user] > 0) return 0; //@audit return 0 for team members
    uint256 initialAllocation = seedAllocations[user];
    if(initialAllocation > 0) {
      if(block.timestamp < vestingStart) return 0;
      uint256 vestPortion = FixedPointMathLib.divWad(block.timestamp - vestingStart, vestingEnd - vestingStart);
      return FixedPointMathLib.mulWad(vestPortion, initialAllocation) - (initialAllocation - stakedLocks[user]);
    }
    else {
      return amount;
    }
  }
HIGH severity: In `GovLocks`, it shouldn't use a `deposits` mapping. **Severity:** High

**Description:** In `GovLocks`, it tracks every user's deposit amount using a `deposits` mapping.
As users can transfer `govLocks` freely, they might have fewer `deposits` than their `govLocks` balance and wouldn't be able to withdraw when they want.

Here is a possible...

Vulnerable code:
function deposit(uint256 amount) external {
    deposits[msg.sender] += amount; //@audit no need
    _moveDelegates(address(0), delegates[msg.sender], amount);
    SafeTransferLib.safeTransferFrom(locks, msg.sender, address(this), amount);
    _mint(msg.sender, amount);
  }

  /// @notice Withdraws Locks to burn Govlocks
  /// @param amount Amount of Locks to withdraw
  function withdraw(uint256 amount) external {
    deposits[msg.sender] -= amount; //@audit no need
    _moveDelegates(delegates[msg.sender], address(0), amount);
    _burn(msg.sender, amount);
    SafeTransferLib.safeTransfer(locks, msg.sender, amount);
  }
HIGH severity: Some functions of `Goldilend` will revert forever.. **Severity:** High

**Description:** `Goldilend.multisigInterestClaim()/apdaoInterestClaim()/sunsetProtocol()` will revert forever because they doesn't withdraw `ibgt` from `ibgtVault` before the transfer.

As `ibgtVault` has all `ibgt` of `Goldilend`, they should withdraw from `ibgtVault`...

Vulnerable code:
function multisigInterestClaim() external {
    if(msg.sender != multisig) revert NotMultisig();
    uint256 interestClaim = multisigClaims;
    multisigClaims = 0;
    SafeTransferLib.safeTransfer(ibgt, multisig, interestClaim);
  }

  /// @inheritdoc IGoldilend
  function apdaoInterestClaim() external {
    if(msg.sender != apdao) revert NotAPDAO();
    uint256 interestClaim = apdaoClaims;
    apdaoClaims = 0;
    SafeTransferLib.safeTransfer(ibgt, apdao, interestClaim);
  }

...

  function sunsetProtocol() external {
    if(msg.sender != timelock) revert NotTimelock();
    SafeTransferLib.safeTransfer(ibgt, multisig, poolSize - outstandingDebt);
  }

function multisigInterestClaim() external {
    if(msg.sender != multisig) revert NotMultisig();
    uint256 interestClaim = multisigClaims;
    multisigClaims = 0;
+  iBGTVault(ibgtVault).withdraw(interestClaim);
    SafeTransferLib.safeTransfer(ibgt, multisig, interestClaim);
  }

  /// @inheritdoc IGoldilend
  function apdaoInterestClaim() external {
    if(msg.sender != apdao) revert NotAPDAO();
    uint256 interestClaim = apdaoClaims;
    apdaoClaims = 0;
+  iBGTVault(ibgtVault).withdraw(interestClaim);
    SafeTransferLib.safeTransfer(ibgt, apdao, interestClaim);
  }

...

  function sunsetProtocol() external {
    if(msg.sender != timelock) revert NotTimelock();
+  iBGTVault(ibgtVault).withdraw(poolSize - outstandingDebt);
    SafeTransferLib.safeTransfer(ibgt, multisig, poolSize - outstandingDebt);
  }