crytic / crytic/slither

For loop index overflows when looping through dynamic array passed as argument to a function

Open
#1,237 0 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Python
Stars
6.4k
Forks
1.1k
PR merge metrics
No merged PRs in 30d

Description

### Describe the desired feature

Consider the following code:

```js
contract Rewards {
mapping(address => uint88) internal userRewards;

modifier onlyOwner() {}

function updateUserRewards(
address[] calldata _userWallets,
uint88[] calldata _userRewards
) external onlyOwner {
for (uint8 i = 0; i < _userWallets.length; i++) {
address userWallet = _userWallets[i];
uint88 userReward = _userRewards[i];

userRewards[userWallet] += userReward;
}
}
}
```

Here's two scripts to create the wallets and rewards arrays:

```js
const { ethers } = require(''ethers')
const { resolve } = require('node:path')
const { randomBytes } = require('node:crypto')
const { writeFile } = require ('node:fs/promises')

async function createRewards(amount:) {
try {
const rewards = []
for (let i = 0; i < amount; i++) {
const randomNumberToString = String(
Math.floor(Math.random() * (10 - 2 + 1) + 2)
)

const ether = randomNumberToString.padEnd(
randomNumberToString.length + 18,
'0'
)

rewards.push(ether)
}

await writeFile(resolve('./rewards.json'), JSON.stringify(rewards))
} catch (err) {
return err
}
}

async function createWallets(amount) {
try {
const wallets = []
for (let i = 0; i < amount; i++) {
const id = randomBytes(32).toString('hex')
const privateKey = `0x${id}`

const wallet = new ethers.Wallet(privateKey)
wallets.push(wallet.address)
}

await writeFile(resolve('./wallets.json'), JSON.stringify(wallets))

console.log(`${amount} wallets were created`)
} catch (err) {
return err
}
}

async function main() {
try {
const amount= Number(process.argv.slice(2)) || 256

await Promise.all([createWallets(amount), createRewards(amount)])
} catch (err) {
return err
}
}

main()
.then(() => process.exit(0))
.catch((err) => {
console.error(err)
process.exit(1)
})
```

If `_userWallets.length` happens to be greater than 2 ** 8 - 1, then the `i` variable will overflow with this EVM error: `VM Exception while processing transaction: reverted with panic code 0x11 (Arithmetic operation underflowed or overflowed outside of an unchecked block`.

I understand that this issue might be very specific to my use case, so feel free to close this issue if the work to add this check isn't worth it.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.