diff --git a/README.md b/README.md index 5c35967..2dac654 100644 --- a/README.md +++ b/README.md @@ -113,6 +113,7 @@ no-bidi-characters | Generic | The code must not contain any of Unicode Directio delegatecall-to-arbitrary-address | Generic | An attacker may perform delegatecall() to an arbitrary address. incorrect-use-of-blockhash | Generic | blockhash(block.number) and blockhash(block.number + N) always returns 0. accessible-selfdestruct | Generic | Contract can be destructed by anyone in $FUNC +missing-array-lengths-check | Generic | User activities may not be completed successfully if the arrays' lengths aren't determined to be the same. ## Gas Optimization Rules diff --git a/solidity/security/missing-array-lengths-check.sol b/solidity/security/missing-array-lengths-check.sol new file mode 100644 index 0000000..03f1c67 --- /dev/null +++ b/solidity/security/missing-array-lengths-check.sol @@ -0,0 +1,76 @@ +pragma solidity ^0.8.0; + +contract ArrayLength { + + function completeQueuedWithdrawals( + //ok: missing-array-lengths-check + QueuedWithdrawal[] calldata queuedWithdrawals, + //ok: missing-array-lengths-check + uint256[] calldata middlewareTimesIndexes, + //ok: missing-array-lengths-check + bytes32[] calldata validatorFields + ) external + { + for(uint256 i = 0; i < queuedWithdrawals.length; i++) { + _completeQueuedWithdrawal(queuedWithdrawals[i], tokens[i], middlewareTimesIndexes[i], receiveAsTokens[i]); + } + + require(queuedWithdrawals.length == middlewareTimesIndexes.length, "different lengths"); + require(middlewareTimesIndexes.length == queuedWithdrawals.length, "different lengths"); + + + } + + + function verifyAndProcessWithdrawal( + BeaconChainProofs.WithdrawalProofs calldata withdrawalProofs, + bytes calldata validatorFieldsProof, + //ruleid: missing-array-lengths-check + bytes32[] calldata validatorFields, + //ruleid: missing-array-lengths-check + bytes32[] calldata withdrawalFields, + //ruleid: missing-array-lengths-check + uint256[] beaconChainETHStrategyIndex, + uint64 oracleBlockNumber + ) + external + { + + uint40 validatorIndex = uint40(Endian.fromLittleEndianUint64(withdrawalFields[BeaconChainProofs.WITHDRAWAL_VALIDATOR_INDEX_INDEX])); + + require(validatorStatus[validatorIndex] != VALIDATOR_STATUS.INACTIVE, + "EigenPod.verifyOvercommittedStake: Validator never proven to have withdrawal credentials pointed to this contract"); + + bytes32 beaconStateRoot = eigenPodManager.getBeaconChainStateRoot(oracleBlockNumber); + + BeaconChainProofs.verifyWithdrawalProofs(beaconStateRoot, withdrawalProofs, withdrawalFields); + BeaconChainProofs.verifyValidatorFields(validatorIndex, beaconStateRoot, validatorFieldsProof, validatorFields); + + uint64 withdrawalAmountGwei = Endian.fromLittleEndianUint64(withdrawalFields[BeaconChainProofs.WITHDRAWAL_VALIDATOR_AMOUNT_INDEX]); + + //check if the withdrawal occured after mostRecentWithdrawalBlockNumber + uint64 slot = Endian.fromLittleEndianUint64(withdrawalProofs.slotRoot); + + (validatorFields[BeaconChainProofs.VALIDATOR_WITHDRAWABLE_EPOCH_INDEX]); + if (Endian.fromLittleEndianUint64(validatorFields[BeaconChainProofs.VALIDATOR_WITHDRAWABLE_EPOCH_INDEX]) <= slot/BeaconChainProofs.SLOTS_PER_EPOCH) { + _processFullWithdrawal(withdrawalAmountGwei, validatorIndex, beaconChainETHStrategyIndex, podOwner, validatorStatus[validatorIndex]); + } else { + _processPartialWithdrawal(slot, withdrawalAmountGwei, validatorIndex, podOwner); + } + } + + function addChainlinkOracles( + //ok: missing-array-lengths-check + address[] memory tokens, + //ok: missing-array-lengths-check + address[] memory oracles, + //ok: missing-array-lengths-check + uint48[] memory heartbeats + ) external { + if (tokens.length != oracles.length || oracles.length != heartbeats.length) { + revert InvalidLength(); + } + // ... + } + +} \ No newline at end of file diff --git a/solidity/security/missing-array-lengths-check.yaml b/solidity/security/missing-array-lengths-check.yaml new file mode 100644 index 0000000..95180f0 --- /dev/null +++ b/solidity/security/missing-array-lengths-check.yaml @@ -0,0 +1,55 @@ +rules: + - id: missing-array-lengths-check + message: User activities may not be completed successfully if the arrays' lengths aren't determined to be the same. + metadata: + category: security + tags: + - array + - length + patterns: + - pattern: | + function $F(..., $TYPE[] $VAR, ..., $TYPE2[] $VAR2, ...) { + ... + } + - pattern-not: | + function $F(..., $TYPE[] $VAR, ..., $TYPE2[] $VAR2, ...) { + ... + require(<... $VAR.length ...>, ...); + ... + } + - pattern-not: | + function $F(..., $TYPE[] $VAR, ..., $TYPE2[] $VAR2, ...) { + ... + require(<... $VAR2.length ...>, ...); + ... + } + - pattern-not: | + function $F(..., $TYPE[] $VAR, ..., $TYPE2[] $VAR2, ...) { + ... + assert(<... $VAR.length ...>, ...); + ... + } + - pattern-not: | + function $F(..., $TYPE[] $VAR, ..., $TYPE2[] $VAR2, ...) { + ... + assert(<... $VAR2.length ...>, ...); + ... + } + - pattern-not: | + function $F(..., $TYPE[] $VAR, ..., $TYPE2[] $VAR2, ...) { + ... + if(<... $VAR.length ...>) {...} + ... + } + - pattern-not: | + function $F(..., $TYPE[] $VAR, ..., $TYPE2[] $VAR2, ...) { + ... + if(<... $VAR2.length ...>){...} + ... + } + - focus-metavariable: + - $VAR + - $VAR2 + languages: + - solidity + severity: INFO \ No newline at end of file