Farseen0/scar-pairs huggingface dataset:
LOW severity: DISCREPANCY IN TRANSACTION RLP DECODING BETWEEN ZKEVM AND EVM. **Severity:** Low

**Path:** [StakeableVestingFactory.sol:deployStakeableVesting#L39-L71](https://github.com/0xPolygonHermez/zkevm-rom/blob/develop/main/load-tx-rlp.zkasm#L164-L206)

**Description:**

In the load-tx-rlp.zkasm implementation of transaction RLP decoding the code label dataREAD...

Vulnerable code:
dataREAD:
        $ => D                          :MLOAD(batchHashPos)
        D                               :MSTORE(dataStarts)
        1 => D
        %CALLDATA_OFFSET => SP          :CALL(addHashTx)
                                        :CALL(addBatchHashData)
        A - 0x80                        :JMPN(veryShortData)
        A - 0x81                        :JMPN(endData)
        A - 0xb8                        :JMPN(shortData)
        A - 0xc0                        :JMPN(longData, invalidTxRLP)

veryShortData:
        1                               :MSTORE(txCalldataLen)
        31 => D                         :CALL(SHLarith)
        A                               :MSTORE(SP++), JMP(endData)

shortData:
        $ => D                          :MLOAD(batchHashPos)
        D                               :MSTORE(dataStarts)
        A - 0x80 => B                   :MSTORE(txCalldataLen), JMP(readData)

longData:
        A - 0xb7 => D                   :CALL(addHashTx)
                                        :CALL(addBatchHashData)
        $ => D                          :MLOAD(batchHashPos)
        D                               :MSTORE(dataStarts)
        A => B                          :MSTORE(txCalldataLen)

readData:
        ; check binaries
        32 => D
        B - D                           :JMPN(readDataFinal)
        B - D                           :MSTORE(txDataRead), CALL(addHashTx)
        A                               :MSTORE(SP++), CALL(addBatchHashByteByByte)
        $ => B                          :MLOAD(txDataRead), JMP(readData)

readDataFinal:
        B - 1                           :JMPN(endData)
        B => D                          :CALL(addHashTx)
        32 - D => D                     :CALL(SHLarith)
        A                               :MSTORE(SP)
        32 - D => D                     :CALL(addBatchHashByteByByte)

case b < 0xC0:
   // If a string is more than 55 bytes long, the RLP encoding consists of a
   // single byte with value 0xB7 plus the length of the length of the
   // string in binary form, followed by the length of the string, followed
   // by the string. For example, a length-1024 string would be encoded as
   // 0xB90400 followed by the string. The range of the first byte is thus
   // [0xB8, 0xBF].
   size, err = s.readUint(b - 0xB7)
   if err == nil && size < 56 {
       err = ErrCanonSize
   }
   return String, size, err
LOW severity: GASLIMIT AND CHAINID MAX SIZE DIFFERENCE BETWEEN ZKEVM AND EVM. **Severity:** Low

**Path:** [StakeableVestingFactory.sol:deployStakeableVesting:L39-71](https://github.com/0xPolygonHermez/zkevm-rom/blob/develop/main/load-tx-rlp.zkasm)

**Description:** 

There is a difference between the transaction’s gas limit and chain id max sizes between zkEVM and EVM...

Vulnerable code:
;; Read RLP 'gas limit'
       ; 256 bits max
gasLimitREAD:
       1 => D                          :CALL(addHashTx)
                                       :CALL(addBatchHashData)
       A - 0x80                        :JMPN(endGasLimit)
       A - 0x81                        :JMPN(gasLimit0)
       A - 0xa1                        :JMPN(shortGasLimit, invalidTxRLP)

;; Read RLP 'chainId'
       ; 64 bits max
chainREAD:
       1 => D                          :CALL(addHashTx)
                                       :CALL(addBatchHashData)
       A - 0x80                        :JMPN(endChainId)
       A - 0x81                        :JMPN(chainId0)
       A - 0x89                        :JMPN(shortChainId, invalidTxRLP)

// LegacyTx is the transaction data of regular Ethereum transactions.
type LegacyTx struct {
   Nonce    uint64          // nonce of sender account
   GasPrice *big.Int        // wei per gas
   Gas      uint64          // gas limit
   To       *common.Address `rlp:"nil"` // nil means contract creation
   Value    *big.Int        // wei amount
   Data     []byte          // contract invocation input data
   V, R, S  *big.Int        // signature values
}
LOW severity: RECOMMENDATION TO CHANGE A CONDITIONAL JUMP. **Severity:** Low

**Path:** /main/opcodes/block.zkasm

**Description:**  

While analysing the zkEVM ROM opcodes and the appropriate PIL representation in the state machines it has been noticed that some of the conditional jumps differ in their operational register size:
-   *JMPN (jump...

Vulnerable code:
/// op0 check zero
pol commit op0Inv;
pol op0IsZero = 1 - op0*op0Inv;
op0IsZero*op0 = 0;
...
pol doJMP = JMPN*isNeg + JMP + JMPC*carry + JMPZ*op0IsZero + return + call;

opBLOCKHASH:
...
   ; Get last tx count
   $ => B          :MLOAD(txCount)
   $ => A          :MLOAD(SP) ; [blockNumber => A]
   ; Check batch block is lt current block number, else return 0
   B - A - 1       :JMPN(opBLOCKHASHzero)
LOW severity: LOOP OPTIMISATION. **Severity:** Low

**Path:** PolygonZkEvm.sol

**Description:** 

In the function updateBatchFee the function calculates number of blocks that were verified above and below the verification time target. The fee is updated based corresponding to their ratio.

The loop that checks for targets that...

Vulnerable code:
function _updateBatchFee(uint64 newLastVerifiedBatch) internal {
   uint64 currentLastVerifiedBatch = getLastVerifiedBatch();
   uint64 currentBatch = newLastVerifiedBatch;uint256 totalBatchesAboveTarget;
uint256 newBatchesVerified = newLastVerifiedBatch -
   currentLastVerifiedBatch;

while (currentBatch != currentLastVerifiedBatch) {
   // Load sequenced batchdata
   SequencedBatchData
       storage currentSequencedBatchData = sequencedBatches[
           currentBatch
       ];

   // Check if timestamp is above or below the VERIFY_BATCH_TIME_TARGET
   if (
       block.timestamp - currentSequencedBatchData.sequencedTimestamp >
       veryBatchTimeTarget
   ) {
       totalBatchesAboveTarget +=
           currentBatch -
           currentSequencedBatchData.previousLastBatchSequenced;   //AUDIT: change to currentBatch - currentLastVerifiedBatch and break out of the loop
   }

   // update currentLastVerifiedBatch
   currentBatch = currentSequencedBatchData.previousLastBatchSequenced;
}
LOW severity: INCORRECT INDEX SIZE IN VERIFYMERKLEPROOF. **Severity:** Informational

**Path:** DepositContract.sol:L90-L112

**Description:**  

In the contract DepositContract.sol
The index is of size uint64, and as the merkle tree levels are 32, the most significant bits of the index can be manipulated to double spend with the same index.

As the...

Vulnerable code:
function verifyMerkleProof(
       bytes32 leafHash,
       bytes32[] memory smtProof,
       uint64 index,
       bytes32 root
   ) public pure returns (bool) {
       bytes32 node = leafHash;

       // Check merkle proof
       uint256 currrentIndex = index; 
       for (
           uint256 height = 0;
           height < _DEPOSIT_CONTRACT_TREE_DEPTH;
           height++
       ) {
           if ((currrentIndex & 1) == 1)
               node = keccak256(abi.encodePacked(smtProof[height], node));
           else node = keccak256(abi.encodePacked(node, smtProof[height]));
           currrentIndex /= 2;
       }

       return node == root;
   }
HIGH severity: REDUNDANT IMPORTS. **Severity:** Informational

**Path:** PolygonZkEVM.sol

**Description:** 

The imports for ERC20BurnableUpgradeable.sol and Initializable.sol are redundant as they are either not used or already imported recursively.

**Remediation:**  Remove the redundant imports.

**Status:** Fixed

- - -

Vulnerable code:
import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol";
//import "@openzeppelin/contracts-upgradeable/token/ERC20/extensions/ERC20BurnableUpgradeable.sol";   // redundant import
import "./interfaces/IVerifierRollup.sol";
import "./interfaces/IPolygonZkEVMGlobalExitRoot.sol";
//import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; // redundant import
import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import "./interfaces/IPolygonZkEVMBridge.sol";
import "./lib/EmergencyManager.sol";
HIGH severity: PLOOKUP AND PERMUTATION SELECTOR POLYNOMIAL DISCREPANCY BETWEEN VERIFYPIL TOOLING AND STARK GENERATION. **Severity:** Informational

**Path:**  [PolygonZkEVM.sol](https://github.com/0xPolygonHermez/pilcom/blob/107765f18d78a865c12517c9c84cbdfbad1d99d0/src/pil_verifier.js#L194-L222)

**Description:** 

In PIL language there is a selector polynomial for choosing specific lines to meet the...

Vulnerable code:
constant %N = 4;

namespace Global(%N);
   pol constant L1;

namespace PermutationExample(%N);

   pol constant b1, b2; // selector polynomials
   pol commit a1, a2;
   b1{a1} is b2{a2};

const b1 = [0,5,0,0];
const b2 = [6,0,0,0];

const a1 = [4,2,3,21];
const a2 = [2,6,19,7];

for (let j=0; j<N; j++) {
   if ((pi.selT==null) || (!F.isZero(pols.exps[pi.selT].v_n[j]))) {
       const vals = []
       for (let k=0; k<pi.t.length; k++) {
           vals.push(F.toString(pols.exps[pi.t[k]].v_n[j]));
       }
       const v = vals.join(",");
       t[v] = (t[v] || 0) + 1;
   }
}

for (let j=0; j<N; j++) {
   if ((pi.selF==null) || (!F.isZero(pols.exps[pi.selF].v_n[j]))) {
       const vals = []
       for (let k=0; k<pi.f.length; k++) {
           vals.push(F.toString(pols.exps[pi.f[k]].v_n[j]));
       }
       const v = vals.join(",");
       const found = t[v] ?? false;
       if (!t[v]) {
           res.push(`${pi.fileName}:${pi.line}:  permutation not `+(found === 0 ? 'enought ':'')+`found w=${j} values: ${v}`);
           console.log(res[res.length-1]);
           if (!config.continueOnError) j=N;  // Do not continue checking
       }
       else {
           t[v] -= 1;
       }
   }
}
HIGH severity: [FTN-4] Activity scores abuse. **Severity:** Critical

**Path:** core/vm/evm.go/memoryGas()

**Description:** `calldata` is counted in two different ways: when sending a transaction from an EOA to a contract, and when sending a transaction from a contract to a contract. Function `memoryGas() `does not take this fact into...

Vulnerable code:
func (evm *EVM) memoryGas(data []byte) (uint64, error) {
    var gas uint64
    rules := evm.ChainConfig().Rules(evm.Context.BlockNumber, evm.Context.Random != nil)
    if len(data) > 0 {
        // Zero and non-zero bytes are priced differently
        var nz uint64
        for _, byt := range data {
            if byt != 0 {
                nz++
            }
        }
        // Make sure we don't exceed uint64 for all data combinations
        nonZeroGas := params.TxDataNonZeroGasFrontier
        if rules.IsIstanbul {
            nonZeroGas = params.TxDataNonZeroGasEIP2028
        }
        if (math.MaxUint64-gas)/nonZeroGas < nz {
            return 0, ErrGasUintOverflow
        }
        gas += nz * nonZeroGas

        z := uint64(len(data)) - nz
        if (math.MaxUint64-gas)/params.TxDataZeroGas < z {
            return 0, ErrGasUintOverflow
        }
        gas += z * params.TxDataZeroGas
    }

    return gas, nil
}
MEDIUM severity: [FTN-3] Incorrectly adding a validator contract. **Severity:** Medium

**Path:** beacon-chain/core/altair/deposit.go/ProcessDeposit()

**Description:** When adding a validator’s contract, if the contract exists, a new contract object with a zero address is added.

The addition of a null contract is due to the fact that the check for the...

Vulnerable code:
func ProcessDeposit(beaconState state.BeaconState, deposit *ethpb.Deposit, verifySignature bool) (state.BeaconState, bool, error) {
    [...]
    index, ok := beaconState.ValidatorIndexByPubkey(bytesutil.ToBytes48(pubKey))
    if !ok {
        [...]
        if contractExist {
            contracts = [][]byte{make([]byte, 20)}
        }
        if err := beaconState.AppendContracts(&ethpb.ContractsContainer{
            Contracts: contracts,
        }); err != nil {
            return nil, newValidator, err
        }
    } else {
        [...]
    }

    return beaconState, newValidator, nil
}

func ProcessDeposit(beaconState state.BeaconState, deposit *ethpb.Deposit, verifySignature bool) (state.BeaconState, bool, error) {
    [...]
    index, ok := beaconState.ValidatorIndexByPubkey(bytesutil.ToBytes48(pubKey))
    if !ok {
        [...]
        if contractExist {
            contracts = [][]byte{make([]byte, 20)}
        } else {
            err := beaconState.AppendContracts(&ethpb.ContractsContainer{Contracts: contracts,})
            if err != nil {
                return nil, newValidator, err
            }
        }
    } else {
        [...]
    }

    return beaconState, newValidator, nil
}
LOW severity: [FTN-6] Hardcoded value instead of constant variable. **Severity:** Low

**Path:** beacon-chain/core/epoch/epoch_processing.go:ProcessTransactionsGasPerPeriodUpdate()

**Description:** Function `ProcessTransactionsGasPerPeriodUpdate()` calculates the gas update transaction per period. In the calculation of gas for the period, it is multiplied by the...

Vulnerable code:
func ProcessTransactionsGasPerPeriodUpdate(state state.BeaconState) (state.BeaconState, error) {
    period := uint64(params.BeaconConfig().EpochsPerActivityPeriod)
    txsPerPeriod := ((state.TransactionsGasPerPeriod()+state.TransactionsPerLatestEpoch()*21000)*period - state.TransactionsGasPerPeriod()) / periodc
    if err := state.SetTransactionsGasPerPeriod(txsPerPeriod); err != nil {
        return nil, err
    }
    if err := state.SetTransactionsPerLatestEpoch(0); err != nil {
        return nil, err
    }

    return state, nil
}
LOW severity: [FTN-9] Code optimisation. **Severity:** Low

**Path:** core/state_transition.go:TransitionDb()

**Description:** Function `TransitionDb()` will transition the state by applying the current message and returning the evm execution result. During the execution of the `TransitionDb()`, activity scores are logged before and...

Vulnerable code:
func (st *StateTransition) TransitionDb() (*ExecutionResult, error) {
    [...]
    
    for _, act := range st.state.GetCurrentActivities() {
        log.Debug("Activities before refund", "address", act.Address, "activity", act.Activity, "delta", act.DeltaActivity) // @note logs in for loops
    }

    [...]
    
    for _, act := range st.state.GetCurrentActivities() {
        log.Debug("Activities after refund", "address", act.Address, "activity", act.Activity, "delta", act.DeltaActivity)
    }

    [...]
}
LOW severity: [LID-1] Stealing ETH using discount factor bypass. **Severity:** Critical

**Path:** WithdrawalQueueBase.sol:_claimWithdrawalTo#L428-L462

**Description:**

Whenever a batch of withdrawal requests is finalised, a discount factor is calculated and a checkpoint is created if the new factor differs. The discount factor is calculated as the actual...

Vulnerable code:
if (_hint + 1 <= lastCheckpointIndex) {
    if (_getCheckpoints()[_hint + 1].fromId <= _hint) {
        revert InvalidHint(_hint);
    }
}
LOW severity: [LID-17] Withdrawal request finalisation will always revert. **Severity:** Critical

**Path:** OracleReportSanityChecker.sol:_checkRequestIdToFinalizeUpTo#L571-L581

**Description:**

A withdrawal request gets finalised through an oracle report from the AccountingOracle to the Lido contract. Lido in turn checks the report’s data using the...

Vulnerable code:
function getWithdrawalRequestStatus(uint256 _requestId)
    external
    view
    returns (WithdrawalRequestStatus memory)
{
    return _getStatus(_requestId);
}
LOW severity: [LID-20] Share rate can be negatively influenced due to shares burn limit. **Severity:** High

**Path:** PositiveTokenRebaseLimiter.sol:getSharesToBurnLimit#L119-L136

**Description:**

The PositiveTokenRebaseLimiter is responsible for limiting the increase of the StETH share rate in the positive direction. For example, if a lot of rewards come in from one oracle report,.....

Vulnerable code:
rebaseLimit = requestedETH / totalETH
sharesBurnLimit = totalShares * (rebaseLimit / (1 + rebaseLimit))

function getSharesToBurnLimit(TokenRebaseLimiterData memory _limiterState)
    internal
    pure
    returns (uint256 maxSharesToBurn)
{
    if (_limiterState.rebaseLimit == UNLIMITED_REBASE) {
        return _limiterState.totalShares;
    }

    uint256 remainingRebase = _limiterState.rebaseLimit - _limiterState.accumulatedRebase;
    maxSharesToBurn = (
        _limiterState.totalShares * remainingRebase
    ) / (LIMITER_PRECISION_BASE + remainingRebase);
}
LOW severity: [LID-2] Bad actor can block Lido deposit function. **Severity:** High

**Path:** Lido.sol:deposit (L696-730), StakingRouter.sol:receive#L153-L155

**Description:**

Using the `Lido:deposit` (L696-730), the Lido protocol makes deposits to the Ethereum 2.0 `DepositContract` using the `StakingRouter` contract, which is inherited from...

Vulnerable code:
function deposit(uint256 _maxDepositsCount, uint256 _stakingModuleId, bytes _depositCalldata) external {
    ILidoLocator locator = getLidoLocator();

    require(msg.sender == locator.depositSecurityModule(), "APP_AUTH_DSM_FAILED");
    require(_stakingModuleId <= uint24(-1), "STAKING_MODULE_ID_TOO_LARGE");
    require(canDeposit(), "CAN_NOT_DEPOSIT");

    uint256 depositableEth = getDepositableEther();

    if (depositableEth > 0) {
        /// available ether amount for deposits (multiple of 32eth)
        depositableEth = Math256.min(depositableEth, _maxDepositsCount.mul(DEPOSIT_SIZE));

        uint256 unaccountedEth = _getUnaccountedEther();
        /// @dev transfer ether to SR and make deposit at the same time
        /// @notice allow zero value of depositableEth, in this case SR will simply transfer the unaccounted ether to Lido contract
        uint256 depositsCount = IStakingRouter(locator.stakingRouter()).deposit.value(depositableEth)(
            _maxDepositsCount,
            _stakingModuleId,
            _depositCalldata
        );

        uint256 depositedAmount = depositsCount.mul(DEPOSIT_SIZE);
        assert(depositedAmount <= depositableEth);

        if (depositsCount > 0) {
            uint256 newDepositedValidators = DEPOSITED_VALIDATORS_POSITION.getStorageUint256().add(depositsCount);
            DEPOSITED_VALIDATORS_POSITION.setStorageUint256(newDepositedValidators);
            emit DepositedValidatorsChanged(newDepositedValidators);

            _markAsUnbuffered(depositedAmount);
            assert(_getUnaccountedEther() == unaccountedEth);
        }
    }
}
LOW severity: [LID-12] Lido does not check actual amount from LidoExecutionLayerRewardsVault rewards withdrawal. **Severity:** Medium

**Path:** Lido.sol:_collectRewardsAndProcessWithdrawals#L827-L860

**Description:**

Part of the Oracle report processing, is the withdrawal of rewards from the LidoExecutionLayerRewardsVault, which happens in `_collectRewardsAndProcessWithdrawals` on line 835-837. 

The...

Vulnerable code:
if (_elRewardsToWithdraw > 0) {
    ILidoExecutionLayerRewardsVault(_contracts.elRewardsVault).withdrawRewards(_elRewardsToWithdraw);
}

function withdrawRewards(uint256 _maxAmount) external returns (uint256 amount) {
    require(msg.sender == LIDO, "ONLY_LIDO_CAN_WITHDRAW");

    uint256 balance = address(this).balance;
    amount = (balance > _maxAmount) ? _maxAmount : balance;
    if (amount > 0) {
        ILido(LIDO).receiveELRewards{value: amount}();
    }
    return amount;
}

_elRewardsToWithdraw = ILidoExecutionLayerRewardsVault(_contracts.elRewardsVault).withdrawRewards(_elRewardsToWithdraw);
LOW severity: [LID-5] Deposit call data not included in guardian signature. **Severity:** Medium

**Path:** DepositSecurityModule.sol:depositBufferedEther#L413-L439

**Description:**

The function `depositBufferedEther` is used to make the Lido contract deposit ETH to the deposit contract and create new nodes. For this a set of valid guardian signatures is...

Vulnerable code:
function depositBufferedEther(
    uint256 blockNumber,
    bytes32 blockHash,
    bytes32 depositRoot,
    uint256 stakingModuleId,
    uint256 nonce,
    bytes calldata depositCalldata,
    Signature[] calldata sortedGuardianSignatures
) external validStakingModuleId(stakingModuleId) {
    if (quorum == 0 || sortedGuardianSignatures.length < quorum) revert DepositNoQuorum();

    bytes32 onchainDepositRoot = IDepositContract(DEPOSIT_CONTRACT).get_deposit_root();
    if (depositRoot != onchainDepositRoot) revert DepositRootChanged();

    if (!STAKING_ROUTER.getStakingModuleIsActive(stakingModuleId)) revert DepositInactiveModule();

    uint256 lastDepositBlock = STAKING_ROUTER.getStakingModuleLastDepositBlock(stakingModuleId);
    if (block.number - lastDepositBlock < minDepositBlockDistance) revert DepositTooFrequent();
    if (blockHash == bytes32(0) || blockhash(blockNumber) != blockHash) revert DepositUnexpectedBlockHash();

    uint256 onchainNonce = STAKING_ROUTER.getStakingModuleNonce(stakingModuleId);
    if (nonce != onchainNonce) revert DepositNonceChanged();

    _verifySignatures(depositRoot, blockNumber, blockHash, stakingModuleId, nonce, sortedGuardianSignatures);

    LIDO.deposit(maxDepositsPerBlock, stakingModuleId, depositCalldata);
}

function _verifySignatures(
    bytes32 depositRoot,
    uint256 blockNumber,
    bytes32 blockHash,
    uint256 stakingModuleId,
    uint256 nonce,
    Signature[] memory sigs
) internal view {
    bytes32 msgHash = keccak256(
        abi.encodePacked(ATTEST_MESSAGE_PREFIX, blockNumber, blockHash, depositRoot, stakingModuleId, nonce)
    );

    address prevSignerAddr = address(0);

    for (uint256 i = 0; i < sigs.length; ++i) {
        address signerAddr = ECDSA.recover(msgHash, sigs[i].r, sigs[i].vs);
        if (!_isGuardian(signerAddr)) revert InvalidSignature();
        if (signerAddr <= prevSignerAddr) revert SignatureNotSorted();
        prevSignerAddr = signerAddr;
    }
}
MEDIUM severity: [LID-21] Withdrawals accrued rewards are not distributed among stakers. **Severity:** Medium

**Path:** PositiveTokenRebaseLimiter.sol

**Description:**

The PositiveTokenRebaseLimiter calculates the rewards to be taken from the vaults and the amount of shares to be burned for withdrawal requests.

OracleReportSanityChecker initialises the PositiveTokenRebaseLimiter...

Vulnerable code:
sharesBurnLimit = (totalWithdrawalRequestETH / totalPooledETH) * totalShares

TokenRebaseLimiterData memory tokenRebaseLimiter = PositiveTokenRebaseLimiter.initLimiterState(
    getMaxPositiveTokenRebase(),
    _preTotalPooledEther,
    _preTotalShares
);
LOW severity: [LID-15] Inactive node operator can always block deposits. **Severity:** Medium

**Path:** NodeOperatorRegistry.sol:_increaseValidatorsKeysNonce#L1315-L1321

**Description:**

The function to deposit ETH to the DepositContract requires a set of signatures of the deposit data. One element is the staking module nonce, which has to be equal to the current...

Vulnerable code:
**Remediation:**  We would recommend to modify `_onlyNodeOperatorManager` such that the node operator manager can only call these kind of functions if the node operator is also active.

For example: